Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection

2023-03-06 Thread Pali Rohár
On Monday 06 March 2023 20:15:07 Tony Dinh wrote:
> Hi Pali,
> 
> On Mon, Mar 6, 2023 at 4:11 PM Pali Rohár  wrote:
> >
> > On Monday 06 March 2023 16:01:58 Tony Dinh wrote:
> > > Hi Pali,
> > >
> > > On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh  wrote:
> > > >
> > > > Hi Pali,
> > > >
> > > > On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár  wrote:
> > > > >
> > > > > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote:
> > > > > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh  wrote:
> > > > > > >
> > > > > > > Hi Pali,
> > > > > > >
> > > > > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár  wrote:
> > > > > > > >
> > > > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote:
> > > > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár  
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Improve code for checking strapping pins which specifies 
> > > > > > > > > > boot mode source.
> > > > > > > > > >
> > > > > > > > > > Martin, could you test if Clearfog can be still configured 
> > > > > > > > > > into UART
> > > > > > > > > > booting mode via HW switches and if it still works 
> > > > > > > > > > correctly? First
> > > > > > > > > > patch is reverting UART related commit for Clearfog which I 
> > > > > > > > > > think it not
> > > > > > > > > > needed anymore.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before 
> > > > > > > > > the switch that
> > > > > > > > > you refactored in cpu.c/get_boot_device is all that gets 
> > > > > > > > > processed. It
> > > > > > > > > decides there is an error and returns BOOT_DEVICE_UART, 
> > > > > > > > > probably because of
> > > > > > > > > the invalid boot workaround for broken UART selection that 
> > > > > > > > > you identified.
> > > > > > > >
> > > > > > > > Ok, so I figured out correctly how this invalid mode works.
> > > > > > > >
> > > > > > > > > UART only works if I use the clearfog_spi_defconfig or if I 
> > > > > > > > > select
> > > > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with 
> > > > > > > > > the MMC or SATA
> > > > > > > > > defconfigs. I get the same result without this patch series 
> > > > > > > > > applied, though.
> > > > > > > > >
> > > > > > > > > The failed cases have the same output (other than kwboot 
> > > > > > > > > header patching
> > > > > > > > > output) until after sending boot image data is complete. The 
> > > > > > > > > output stops
> > > > > > > > > after:
> > > > > > > > > 
> > > > > > > > >  98 % 
> > > > > > > > > [.
> > > > > > > > >   ]
> > > > > > > > > Done
> > > > > > > > > Finishing transfer
> > > > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > > > 
> > > > > > > >
> > > > > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART 
> > > > > > > > just
> > > > > > > > instruct mkimage what to put into kwbimage header.
> > > > > > > >
> > > > > > > > If I'm looking at the output correctly then SPL was booted, it 
> > > > > > > > correctly
> > > > > > > > trained DDR RAM, returned back to bootrom, kwboot continued 
> > > > > > > > sending main
> > > > > > > > u-boot and bootrom confirmed that transfer of both SPL and main 
> > > > > > > > u-boot
> > > > > > > > is complete. But then there is no output from main u-boot.
> > > > > > > >
> > > > > > > > > It looks like an unrelated issue with kwboot.c, which I was 
> > > > > > > > > sure was
> > > > > > > > > working after the last patches but I can no longer reproduce 
> > > > > > > > > a successful
> > > > > > > > > boot.
> > > > > > > >
> > > > > > > > Can you check that you are using _both_ mkimage and kwboot from 
> > > > > > > > version
> > > > > > > > with applying _all_ my patches recently sent to ML? Because 
> > > > > > > > both mkimage
> > > > > > > > and kwboot have fixes for SATA and SDIO images.
> > > > > > > >
> > > > > > > > For me it looks like that either mkimage generated incorrect 
> > > > > > > > image size
> > > > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image 
> > > > > > > > size
> > > > > > > > from kwbimage header and sent smaller image.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > Also could you check if SATA booting is still working 
> > > > > > > > > > correctly?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > SATA works correctly.
> > > > > > > >
> > > > > > > > Perfect!
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > Tony, should address problems with SPI booting when it is 
> > > > > > > > > > configured to
> > > > > > > > > > different configuration. In fourth commit I added all 
> > > > > > > > > > possible boot mode
> > > > > > > > > > strapping pin configurations which are recognized by A385 
> > > > > > > > > > bootrom (and
> > > > > > > > > > not the only one described in the HW spec, which is 
> > > > > > > > > > incomplete).
> > > > > > >
> > > > > > > It 

[PATCH 2/2] ARM: imx: Enable SDP download in SPL on DH i.MX6 DHSOM

2023-03-06 Thread Marek Vasut
Enable SDP protocol support in SPL for DH i.MX6 DHSOM, now that those
components fit into the SPL due to LTO.

To start U-Boot via SDP upload on i.MX6 DHSOM based board, proceed as follows:
- Compile imx_usb [1] .
- Power off the i.MX6 DHSOM based board.
- Connect both USB-serial console and USB-OTG miniB ports to host PC.
- Switch board to USB boot mode.
- Power on the board.
- Verify using '$ dmesg' that a new device has been detected as follows:
New USB device found, idVendor=15a2, idProduct=0054, bcdDevice= 0.01
New USB device strings: Mfr=1, Product=2, SerialNumber=0
Product: SE Blank ARIK
Manufacturer: Freescale SemiConductor Inc

- Upload U-Boot SPL:
$ imx_usb u-boot-with-spl.imx

- Wait for SPL to come up, the following print ought to be the last on
  UART console:
SDP: handle requests...

- Upload U-Boot proper:
$ imx_usb u-boot.img

[1] https://github.com/boundarydevices/imx_usb_loader.git

Signed-off-by: Marek Vasut 
---
Cc: Andreas Geisreiter 
Cc: Christoph Niedermaier 
Cc: Fabio Estevam 
Cc: NXP i.MX U-Boot Team 
Cc: Stefano Babic 
Cc: u-b...@dh-electronics.com
---
 configs/dh_imx6_defconfig | 5 +
 include/configs/dh_imx6.h | 2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index cf52b0e3a00..3de974f139d 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -40,6 +40,10 @@ CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y
 CONFIG_SYS_SPL_MALLOC=y
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_SYS_SPI_U_BOOT_OFFS=0x11400
+CONFIG_SPL_USB_HOST=y
+CONFIG_SPL_USB_GADGET=y
+CONFIG_SPL_USB_SDP_SUPPORT=y
+CONFIG_SPL_WATCHDOG=y
 CONFIG_SYS_MAXARGS=32
 CONFIG_SYS_PBSIZE=532
 CONFIG_CMD_MEMTEST=y
@@ -113,6 +117,7 @@ CONFIG_USB_GADGET_MANUFACTURER="dh"
 CONFIG_USB_GADGET_VENDOR_NUM=0x0525
 CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5
 CONFIG_CI_UDC=y
+CONFIG_SDP_LOADADDR=0x17c0
 CONFIG_USB_GADGET_DOWNLOAD=y
 CONFIG_WATCHDOG_TIMEOUT_MSECS=6
 CONFIG_IMX_WATCHDOG=y
diff --git a/include/configs/dh_imx6.h b/include/configs/dh_imx6.h
index 5cf73274d5e..ef27e483295 100644
--- a/include/configs/dh_imx6.h
+++ b/include/configs/dh_imx6.h
@@ -31,7 +31,6 @@
 #define CFG_MXC_UART_BASE  UART1_BASE
 
 /* USB Configs */
-#ifdef CONFIG_CMD_USB
 #define CFG_MXC_USB_PORTSC (PORT_PTS_UTMI | PORT_PTS_PTW)
 #define CFG_MXC_USB_FLAGS  0
 
@@ -39,7 +38,6 @@
 #if defined(CONFIG_CMD_DFU) || defined(CONFIG_CMD_USB_MASS_STORAGE)
 #define DFU_DEFAULT_POLL_TIMEOUT   300
 #endif
-#endif
 
 #define CFG_EXTRA_ENV_SETTINGS \
"console=ttymxc0,115200\0"  \
-- 
2.39.2



[PATCH 1/2] ARM: imx: Enable LTO for DH electronics i.MX6 DHCOM

2023-03-06 Thread Marek Vasut
Enable LTO to reduce the size of SPL, which with SPL SDP
support may be close to the limit.

Signed-off-by: Marek Vasut 
---
Cc: Andreas Geisreiter 
Cc: Christoph Niedermaier 
Cc: Fabio Estevam 
Cc: NXP i.MX U-Boot Team 
Cc: Stefano Babic 
Cc: u-b...@dh-electronics.com
---
 configs/dh_imx6_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index 62c446f86a9..cf52b0e3a00 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -28,6 +28,7 @@ CONFIG_SPL_SPI=y
 CONFIG_AHCI=y
 CONFIG_SYS_MEMTEST_START=0x1000
 CONFIG_SYS_MEMTEST_END=0x2000
+CONFIG_LTO=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_SYS_MONITOR_LEN=409600
 CONFIG_FIT=y
-- 
2.39.2



[PATCH] usb: gadget: f_sdp: Add missing spl_board_prepare_for_boot() call

2023-03-06 Thread Marek Vasut
The spl_board_prepare_for_boot() should be called before jump_to_image_no_args()
to perform board-specific deinitialization before jumping to the next stage.
This board-specific deinitialization can be very much anything, e.g. disable
dcache in case it was enabled, or such.

Add the missing spl_board_prepare_for_boot() call into f_sdp .

Signed-off-by: Marek Vasut 
---
Cc: Frieder Schrempf 
Cc: Lukasz Majewski 
Cc: Patrick Delaunay 
Cc: Peng Fan 
Cc: Sean Anderson 
Cc: Sherry Sun 
Cc: Simon Glass 
Cc: Ye Li 
---
 drivers/usb/gadget/f_sdp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c
index 9ea43f29cfb..4da5a160a09 100644
--- a/drivers/usb/gadget/f_sdp.c
+++ b/drivers/usb/gadget/f_sdp.c
@@ -865,6 +865,7 @@ static int sdp_handle_in_ep(struct spl_image_info 
*spl_image,
struct spl_image_info spl_image = {};
struct spl_boot_device bootdev = {};
spl_parse_image_header(_image, , header);
+   spl_board_prepare_for_boot();
jump_to_image_no_args(_image);
 #else
/* In U-Boot, allow jumps to scripts */
-- 
2.39.2



Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree

2023-03-06 Thread Conor Dooley



On 7 March 2023 01:59:31 GMT, yanhong wang  
wrote:
>
>
>On 2023/3/4 5:16, Conor Dooley wrote:
>> On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote:
>>> Add initial device tree for the JH7110 RISC-V SoC.
>>> 
>>> Signed-off-by: Yanhong Wang 
>>> ---
>>>  arch/riscv/dts/jh7110.dtsi | 582 +
>>>  1 file changed, 582 insertions(+)
>>>  create mode 100644 arch/riscv/dts/jh7110.dtsi
>>> 
>>> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
>>> new file mode 100644
>>> index 00..d3e9f92987
>>> --- /dev/null
>>> +++ b/arch/riscv/dts/jh7110.dtsi
>>> @@ -0,0 +1,582 @@
>>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>>> +/*
>>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include 
>>> +#include 
>>> +
>>> +/ {
>>> +   compatible = "starfive,jh7110";
>>> +   #address-cells = <2>;
>>> +   #size-cells = <2>;
>>> +
>>> +   cpus {
>>> +   #address-cells = <1>;
>>> +   #size-cells = <0>;
>>> +
>>> +   S7_0: cpu@0 {
>>> +   compatible = "sifive,s7", "riscv";
>>> +   reg = <0>;
>>> +   d-cache-block-size = <64>;
>>> +   d-cache-sets = <64>;
>>> +   d-cache-size = <8192>;
>>> +   d-tlb-sets = <1>;
>>> +   d-tlb-size = <40>;
>>> +   device_type = "cpu";
>>> +   i-cache-block-size = <64>;
>>> +   i-cache-sets = <64>;
>>> +   i-cache-size = <16384>;
>>> +   i-tlb-sets = <1>;
>>> +   i-tlb-size = <40>;
>>> +   mmu-type = "riscv,sv39";
>>> +   next-level-cache = <>;
>>> +   riscv,isa = "rv64imac_zba_zbb";
>> 
>> Hmm, based on what Sean said on the previous version, "We use strchr on
>> it; so something like Zicsr is parsed as 5 extensions", are you sure that
>> adding this here behaves correctly?
>> 
>
>As you said, u-boot does not parse the content after '_', zba/zbb has no 
>practical meaning in u-boot. 

That's not what Sean's comment on the previous version said.
If it is actually ignored, this is fine, but Sean's comment read like it would 
be misinterpreted by U-Boot.
I'll have to go read the code.

>The definitions of 'riscv,isa' refer to linux and is consistent with this.
>
>https://patchwork.kernel.org/project/linux-riscv/patch/20230221024645.127922-18-hal.f...@starfivetech.com/
>
>
>Do you have any better suggestion?  Referring to the definition of Sifive 
>fu740, remove '_zba_zbb'?

The fu740 doesn't have those extensions.

>
>> https://lore.kernel.org/u-boot/22e805d4-f823-975c-a970-a4a19bb13...@gmail.com/
>> 
>> I know that having zba/zbb is *correct*, but if U-Boot doesn't parse it
>> correctly...
>> 
>> Cheers,
>> Conor.


Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection

2023-03-06 Thread Tony Dinh
Hi Pali,

On Mon, Mar 6, 2023 at 4:11 PM Pali Rohár  wrote:
>
> On Monday 06 March 2023 16:01:58 Tony Dinh wrote:
> > Hi Pali,
> >
> > On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh  wrote:
> > >
> > > Hi Pali,
> > >
> > > On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár  wrote:
> > > >
> > > > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote:
> > > > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh  wrote:
> > > > > >
> > > > > > Hi Pali,
> > > > > >
> > > > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár  wrote:
> > > > > > >
> > > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote:
> > > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár  wrote:
> > > > > > > >
> > > > > > > > > Improve code for checking strapping pins which specifies boot 
> > > > > > > > > mode source.
> > > > > > > > >
> > > > > > > > > Martin, could you test if Clearfog can be still configured 
> > > > > > > > > into UART
> > > > > > > > > booting mode via HW switches and if it still works correctly? 
> > > > > > > > > First
> > > > > > > > > patch is reverting UART related commit for Clearfog which I 
> > > > > > > > > think it not
> > > > > > > > > needed anymore.
> > > > > > > > >
> > > > > > > >
> > > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before the 
> > > > > > > > switch that
> > > > > > > > you refactored in cpu.c/get_boot_device is all that gets 
> > > > > > > > processed. It
> > > > > > > > decides there is an error and returns BOOT_DEVICE_UART, 
> > > > > > > > probably because of
> > > > > > > > the invalid boot workaround for broken UART selection that you 
> > > > > > > > identified.
> > > > > > >
> > > > > > > Ok, so I figured out correctly how this invalid mode works.
> > > > > > >
> > > > > > > > UART only works if I use the clearfog_spi_defconfig or if I 
> > > > > > > > select
> > > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with the 
> > > > > > > > MMC or SATA
> > > > > > > > defconfigs. I get the same result without this patch series 
> > > > > > > > applied, though.
> > > > > > > >
> > > > > > > > The failed cases have the same output (other than kwboot header 
> > > > > > > > patching
> > > > > > > > output) until after sending boot image data is complete. The 
> > > > > > > > output stops
> > > > > > > > after:
> > > > > > > > 
> > > > > > > >  98 % 
> > > > > > > > [.
> > > > > > > >   ]
> > > > > > > > Done
> > > > > > > > Finishing transfer
> > > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > > 
> > > > > > >
> > > > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART 
> > > > > > > just
> > > > > > > instruct mkimage what to put into kwbimage header.
> > > > > > >
> > > > > > > If I'm looking at the output correctly then SPL was booted, it 
> > > > > > > correctly
> > > > > > > trained DDR RAM, returned back to bootrom, kwboot continued 
> > > > > > > sending main
> > > > > > > u-boot and bootrom confirmed that transfer of both SPL and main 
> > > > > > > u-boot
> > > > > > > is complete. But then there is no output from main u-boot.
> > > > > > >
> > > > > > > > It looks like an unrelated issue with kwboot.c, which I was 
> > > > > > > > sure was
> > > > > > > > working after the last patches but I can no longer reproduce a 
> > > > > > > > successful
> > > > > > > > boot.
> > > > > > >
> > > > > > > Can you check that you are using _both_ mkimage and kwboot from 
> > > > > > > version
> > > > > > > with applying _all_ my patches recently sent to ML? Because both 
> > > > > > > mkimage
> > > > > > > and kwboot have fixes for SATA and SDIO images.
> > > > > > >
> > > > > > > For me it looks like that either mkimage generated incorrect 
> > > > > > > image size
> > > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image 
> > > > > > > size
> > > > > > > from kwbimage header and sent smaller image.
> > > > > > >
> > > > > > > >
> > > > > > > > > Also could you check if SATA booting is still working 
> > > > > > > > > correctly?
> > > > > > > > >
> > > > > > > >
> > > > > > > > SATA works correctly.
> > > > > > >
> > > > > > > Perfect!
> > > > > > >
> > > > > > > >
> > > > > > > > > Tony, should address problems with SPI booting when it is 
> > > > > > > > > configured to
> > > > > > > > > different configuration. In fourth commit I added all 
> > > > > > > > > possible boot mode
> > > > > > > > > strapping pin configurations which are recognized by A385 
> > > > > > > > > bootrom (and
> > > > > > > > > not the only one described in the HW spec, which is 
> > > > > > > > > incomplete).
> > > > > >
> > > > > > It works great! Here the strapping is SPI 1 (0x34), and the boot 
> > > > > > mode
> > > > > > is set to "Trying to boot from SPI" correctly.  I'm having a problem
> > > > > > with SPL SPI probing the device. But I don't think it is not related
> > > > > > to this boot mode patch. There is something in SPL 

Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree

2023-03-06 Thread yanhong wang



On 2023/3/4 5:16, Conor Dooley wrote:
> On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote:
>> Add initial device tree for the JH7110 RISC-V SoC.
>> 
>> Signed-off-by: Yanhong Wang 
>> ---
>>  arch/riscv/dts/jh7110.dtsi | 582 +
>>  1 file changed, 582 insertions(+)
>>  create mode 100644 arch/riscv/dts/jh7110.dtsi
>> 
>> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
>> new file mode 100644
>> index 00..d3e9f92987
>> --- /dev/null
>> +++ b/arch/riscv/dts/jh7110.dtsi
>> @@ -0,0 +1,582 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +/*
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + */
>> +
>> +/dts-v1/;
>> +#include 
>> +#include 
>> +
>> +/ {
>> +compatible = "starfive,jh7110";
>> +#address-cells = <2>;
>> +#size-cells = <2>;
>> +
>> +cpus {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +S7_0: cpu@0 {
>> +compatible = "sifive,s7", "riscv";
>> +reg = <0>;
>> +d-cache-block-size = <64>;
>> +d-cache-sets = <64>;
>> +d-cache-size = <8192>;
>> +d-tlb-sets = <1>;
>> +d-tlb-size = <40>;
>> +device_type = "cpu";
>> +i-cache-block-size = <64>;
>> +i-cache-sets = <64>;
>> +i-cache-size = <16384>;
>> +i-tlb-sets = <1>;
>> +i-tlb-size = <40>;
>> +mmu-type = "riscv,sv39";
>> +next-level-cache = <>;
>> +riscv,isa = "rv64imac_zba_zbb";
> 
> Hmm, based on what Sean said on the previous version, "We use strchr on
> it; so something like Zicsr is parsed as 5 extensions", are you sure that
> adding this here behaves correctly?
> 

As you said, u-boot does not parse the content after '_', zba/zbb has no 
practical meaning in u-boot. 
The definitions of 'riscv,isa' refer to linux and is consistent with this.

https://patchwork.kernel.org/project/linux-riscv/patch/20230221024645.127922-18-hal.f...@starfivetech.com/


Do you have any better suggestion?  Referring to the definition of Sifive 
fu740, remove '_zba_zbb'?

> https://lore.kernel.org/u-boot/22e805d4-f823-975c-a970-a4a19bb13...@gmail.com/
> 
> I know that having zba/zbb is *correct*, but if U-Boot doesn't parse it
> correctly...
> 
> Cheers,
> Conor.


Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection

2023-03-06 Thread Pali Rohár
On Monday 06 March 2023 16:01:58 Tony Dinh wrote:
> Hi Pali,
> 
> On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh  wrote:
> >
> > Hi Pali,
> >
> > On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár  wrote:
> > >
> > > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote:
> > > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh  wrote:
> > > > >
> > > > > Hi Pali,
> > > > >
> > > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár  wrote:
> > > > > >
> > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote:
> > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár  wrote:
> > > > > > >
> > > > > > > > Improve code for checking strapping pins which specifies boot 
> > > > > > > > mode source.
> > > > > > > >
> > > > > > > > Martin, could you test if Clearfog can be still configured into 
> > > > > > > > UART
> > > > > > > > booting mode via HW switches and if it still works correctly? 
> > > > > > > > First
> > > > > > > > patch is reverting UART related commit for Clearfog which I 
> > > > > > > > think it not
> > > > > > > > needed anymore.
> > > > > > > >
> > > > > > >
> > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before the 
> > > > > > > switch that
> > > > > > > you refactored in cpu.c/get_boot_device is all that gets 
> > > > > > > processed. It
> > > > > > > decides there is an error and returns BOOT_DEVICE_UART, probably 
> > > > > > > because of
> > > > > > > the invalid boot workaround for broken UART selection that you 
> > > > > > > identified.
> > > > > >
> > > > > > Ok, so I figured out correctly how this invalid mode works.
> > > > > >
> > > > > > > UART only works if I use the clearfog_spi_defconfig or if I select
> > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with the 
> > > > > > > MMC or SATA
> > > > > > > defconfigs. I get the same result without this patch series 
> > > > > > > applied, though.
> > > > > > >
> > > > > > > The failed cases have the same output (other than kwboot header 
> > > > > > > patching
> > > > > > > output) until after sending boot image data is complete. The 
> > > > > > > output stops
> > > > > > > after:
> > > > > > > 
> > > > > > >  98 % 
> > > > > > > [.
> > > > > > >   ]
> > > > > > > Done
> > > > > > > Finishing transfer
> > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > 
> > > > > >
> > > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART just
> > > > > > instruct mkimage what to put into kwbimage header.
> > > > > >
> > > > > > If I'm looking at the output correctly then SPL was booted, it 
> > > > > > correctly
> > > > > > trained DDR RAM, returned back to bootrom, kwboot continued sending 
> > > > > > main
> > > > > > u-boot and bootrom confirmed that transfer of both SPL and main 
> > > > > > u-boot
> > > > > > is complete. But then there is no output from main u-boot.
> > > > > >
> > > > > > > It looks like an unrelated issue with kwboot.c, which I was sure 
> > > > > > > was
> > > > > > > working after the last patches but I can no longer reproduce a 
> > > > > > > successful
> > > > > > > boot.
> > > > > >
> > > > > > Can you check that you are using _both_ mkimage and kwboot from 
> > > > > > version
> > > > > > with applying _all_ my patches recently sent to ML? Because both 
> > > > > > mkimage
> > > > > > and kwboot have fixes for SATA and SDIO images.
> > > > > >
> > > > > > For me it looks like that either mkimage generated incorrect image 
> > > > > > size
> > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image size
> > > > > > from kwbimage header and sent smaller image.
> > > > > >
> > > > > > >
> > > > > > > > Also could you check if SATA booting is still working correctly?
> > > > > > > >
> > > > > > >
> > > > > > > SATA works correctly.
> > > > > >
> > > > > > Perfect!
> > > > > >
> > > > > > >
> > > > > > > > Tony, should address problems with SPI booting when it is 
> > > > > > > > configured to
> > > > > > > > different configuration. In fourth commit I added all possible 
> > > > > > > > boot mode
> > > > > > > > strapping pin configurations which are recognized by A385 
> > > > > > > > bootrom (and
> > > > > > > > not the only one described in the HW spec, which is incomplete).
> > > > >
> > > > > It works great! Here the strapping is SPI 1 (0x34), and the boot mode
> > > > > is set to "Trying to boot from SPI" correctly.  I'm having a problem
> > > > > with SPL SPI probing the device. But I don't think it is not related
> > > > > to this boot mode patch. There is something in SPL SPI that either I
> > > > > don't understand or it is a bug.
> > > >
> > > > I meant "But I don't think it is related to this boot mode patch".
> > >
> > > 0x34 uses SPI controller 1. So maybe you need to adjust some config
> > > options? In your log is usage of bus 0, so maybe this could be the
> > > reason.
> >
> > Previously I did not use bus 1, and used the default bus 0 and 

Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection

2023-03-06 Thread Tony Dinh
Hi Pali,

On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh  wrote:
>
> Hi Pali,
>
> On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár  wrote:
> >
> > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote:
> > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh  wrote:
> > > >
> > > > Hi Pali,
> > > >
> > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár  wrote:
> > > > >
> > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote:
> > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár  wrote:
> > > > > >
> > > > > > > Improve code for checking strapping pins which specifies boot 
> > > > > > > mode source.
> > > > > > >
> > > > > > > Martin, could you test if Clearfog can be still configured into 
> > > > > > > UART
> > > > > > > booting mode via HW switches and if it still works correctly? 
> > > > > > > First
> > > > > > > patch is reverting UART related commit for Clearfog which I think 
> > > > > > > it not
> > > > > > > needed anymore.
> > > > > > >
> > > > > >
> > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before the 
> > > > > > switch that
> > > > > > you refactored in cpu.c/get_boot_device is all that gets processed. 
> > > > > > It
> > > > > > decides there is an error and returns BOOT_DEVICE_UART, probably 
> > > > > > because of
> > > > > > the invalid boot workaround for broken UART selection that you 
> > > > > > identified.
> > > > >
> > > > > Ok, so I figured out correctly how this invalid mode works.
> > > > >
> > > > > > UART only works if I use the clearfog_spi_defconfig or if I select
> > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with the MMC 
> > > > > > or SATA
> > > > > > defconfigs. I get the same result without this patch series 
> > > > > > applied, though.
> > > > > >
> > > > > > The failed cases have the same output (other than kwboot header 
> > > > > > patching
> > > > > > output) until after sending boot image data is complete. The output 
> > > > > > stops
> > > > > > after:
> > > > > > 
> > > > > >  98 % 
> > > > > > [.
> > > > > >   ]
> > > > > > Done
> > > > > > Finishing transfer
> > > > > > [Type Ctrl-\ + c to quit]
> > > > > > 
> > > > >
> > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART just
> > > > > instruct mkimage what to put into kwbimage header.
> > > > >
> > > > > If I'm looking at the output correctly then SPL was booted, it 
> > > > > correctly
> > > > > trained DDR RAM, returned back to bootrom, kwboot continued sending 
> > > > > main
> > > > > u-boot and bootrom confirmed that transfer of both SPL and main u-boot
> > > > > is complete. But then there is no output from main u-boot.
> > > > >
> > > > > > It looks like an unrelated issue with kwboot.c, which I was sure was
> > > > > > working after the last patches but I can no longer reproduce a 
> > > > > > successful
> > > > > > boot.
> > > > >
> > > > > Can you check that you are using _both_ mkimage and kwboot from 
> > > > > version
> > > > > with applying _all_ my patches recently sent to ML? Because both 
> > > > > mkimage
> > > > > and kwboot have fixes for SATA and SDIO images.
> > > > >
> > > > > For me it looks like that either mkimage generated incorrect image 
> > > > > size
> > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image size
> > > > > from kwbimage header and sent smaller image.
> > > > >
> > > > > >
> > > > > > > Also could you check if SATA booting is still working correctly?
> > > > > > >
> > > > > >
> > > > > > SATA works correctly.
> > > > >
> > > > > Perfect!
> > > > >
> > > > > >
> > > > > > > Tony, should address problems with SPI booting when it is 
> > > > > > > configured to
> > > > > > > different configuration. In fourth commit I added all possible 
> > > > > > > boot mode
> > > > > > > strapping pin configurations which are recognized by A385 bootrom 
> > > > > > > (and
> > > > > > > not the only one described in the HW spec, which is incomplete).
> > > >
> > > > It works great! Here the strapping is SPI 1 (0x34), and the boot mode
> > > > is set to "Trying to boot from SPI" correctly.  I'm having a problem
> > > > with SPL SPI probing the device. But I don't think it is not related
> > > > to this boot mode patch. There is something in SPL SPI that either I
> > > > don't understand or it is a bug.
> > >
> > > I meant "But I don't think it is related to this boot mode patch".
> >
> > 0x34 uses SPI controller 1. So maybe you need to adjust some config
> > options? In your log is usage of bus 0, so maybe this could be the
> > reason.
>
> Previously I did not use bus 1, and used the default bus 0 and it
> still works! so I've suspected there is some problem in SPL SPI (i.e.
> it works when it should not). But now default bus 0 no longer works.
>
> I am configuring bus 1,  but I'm not yet successful. There are
> multiple places where bus 1 is needed to be specified. Also, might I
> also need -u-boot.dtsi to 

Re: [PATCH] cmd: fdt: Drop the 0x prefix

2023-03-06 Thread Simon Glass
Hi Marek,

On Mon, 6 Mar 2023 at 12:07, Marek Vasut  wrote:
>
> On 3/6/23 18:53, Simon Glass wrote:
> > Hi Marek,
> >
> > On Wed, 1 Mar 2023 at 20:04, Marek Vasut
> >  wrote:
> >>
> >> The 'fdt get addr' is always assumed to be hex value, drop the prefix.
> >> Since this might break existing users who depend on the existing
> >> behavior with 0x prefix, this is a separate patch. Revert if this
> >> breaks anything.
> >>
> >> Signed-off-by: Marek Vasut 
> >> ---
> >> Cc: Heinrich Schuchardt 
> >> Cc: Simon Glass 
> >> Cc: Tom Rini 
> >> ---
> >>   cmd/fdt.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/cmd/fdt.c b/cmd/fdt.c
> >> index f38fe909c3e..04b664e652c 100644
> >> --- a/cmd/fdt.c
> >> +++ b/cmd/fdt.c
> >> @@ -478,7 +478,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int 
> >> argc, char *const argv[])
> >>  /* Get address */
> >>  char buf[19];
> >>
> >> -   snprintf(buf, sizeof(buf), "0x%lx",
> >> +   snprintf(buf, sizeof(buf), "%lx",
> >>   
> >> (ulong)map_to_sysmem(nodep));
> >>  env_set(var, buf);
> >>  } else if (subcmd[0] == 's') {
> >> --
> >> 2.39.2
> >>
> >
> > iwc how about using env_sethex() ?
>
> The 'env get size' 's' case below could likely use similar treatment ,
> do I read it right ?

Yes...I think the helpers were added more recently than this code.

Regards,
Simon


Re: [PATCH v5 0/6] FWU: Handle meta-data in common code

2023-03-06 Thread Jassi Brar
On Mon, Feb 27, 2023 at 7:28 PM Tom Rini  wrote:
>
> On Mon, Feb 27, 2023 at 07:00:10PM -0600, Jassi Brar wrote:
> > On Mon, 27 Feb 2023 at 18:58, Tom Rini  wrote:
> > >
> > > On Mon, Feb 27, 2023 at 06:51:35PM -0600, jassisinghb...@gmail.com wrote:
> > >
> > > > From: Jassi Brar 
> > > >
> > > > The patchset reduces ~400 lines of code, while keeping the 
> > > > functionality same and making
> > > > meta-data operations much faster (by using cached structures).
> > > >
> > > > Issue:
> > > >  meta-data copies (primary and secondary) are being handled by the 
> > > > backend/storage layer
> > > > instead of the common core in fwu.c (as also noted by Ilias)  that is, 
> > > > gpt_blk.c manages
> > > > meta-data and similarly raw_mtd.c will have to do the same when it 
> > > > arrives. The code
> > > > could by make smaller, cleaner and optimised.
> > > >
> > > > Basic idea:
> > > >  Introduce  .read_mdata() and .write_mdata() in fwu_mdata_ops  that 
> > > > simply read/write
> > > > meta-data copy. The core code takes care of integrity and redundancy of 
> > > > the meta-data,
> > > > as a result we can get rid of every other callback .get_mdata() 
> > > > .update_mdata()
> > > > .get_mdata_part_num()  .read_mdata_partition()  
> > > > .write_mdata_partition() and the
> > > > corresponding wrapper functions thereby making the code 100s of LOC 
> > > > smaller.
> > > >
> > > > Get rid of fwu_check_mdata_validity() and fwu_mdata_check() which 
> > > > expected underlying
> > > > layer to manage and verify mdata copies.
> > > > Implement  fwu_get_verified_mdata(struct fwu_mdata *mdata) public 
> > > > function that reads,
> > > > verifies and, if needed, fixes the meta-data copies.
> > > >
> > > > Verified copy of meta-data is now cached as 'g_mdata' in fwu.c, which 
> > > > avoids multiple
> > > > low-level expensive read and parse calls.
> > > > gpt meta-data partition numbers are now cached in gpt_blk.c, so that we 
> > > > don't have to do expensive part_get_info() and uid ops.
> > > >
> > > > Changes since v4:
> > > > * Change fwu-mdata-mtd bindings to not require external changes
> > > > * Handle 'part == BOTH_PARTS' in fwu_sync_mdata
> > > > * use parts_ok[] and parts_mdata[] instead of pri/sec_ok and 
> > > > p/s_mdata
> > >
> > > Did you run this through CI / build sandbox? This doesn't read like you
> > > fixed the problem I reported in CI, when I was trying to merge v4.
> > >
> > I know that remains to be done.
> > The dt-bindings for fwu-mdata is changed in this patchset and I
> > thought any testcase may be impacted by it.
>
> So you're not expecting this iteration to be merged, as CI doesn't pass,
> and that's known? OK.
>
Sorry I got confused. I thought new test cases are expected to be
written, whereas you only wanted the sandbox compilation breakage
fixed. My replies must have sounded so stupid :(
I have a patch to fix sandbox compilation in the v6 revision.

thanks.


[PATCH v6 7/7] test: dm: fwu: fix for the updated api

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

fwu_get_mdata() no more requires 'dev' argument and
fwu_check_mdata_validity() has been rendered useless and dropped.
Fix the test cases to work with aforementioned changes.

Signed-off-by: Jassi Brar 
---
 test/dm/fwu_mdata.c | 22 +++---
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/test/dm/fwu_mdata.c b/test/dm/fwu_mdata.c
index b179a65c15..8b5c83ef4e 100644
--- a/test/dm/fwu_mdata.c
+++ b/test/dm/fwu_mdata.c
@@ -98,7 +98,7 @@ static int dm_test_fwu_mdata_read(struct unit_test_state *uts)
ut_assertok(populate_mmc_disk_image(uts));
ut_assertok(write_mmc_blk_device(uts));
 
-   ut_assertok(fwu_get_mdata(dev, ));
+   ut_assertok(fwu_get_mdata());
 
ut_asserteq(mdata.version, 0x1);
 
@@ -118,30 +118,14 @@ static int dm_test_fwu_mdata_write(struct unit_test_state 
*uts)
 
ut_assertok(uclass_first_device_err(UCLASS_FWU_MDATA, ));
 
-   ut_assertok(fwu_get_mdata(dev, ));
+   ut_assertok(fwu_get_mdata());
 
active_idx = (mdata.active_index + 1) % CONFIG_FWU_NUM_BANKS;
ut_assertok(fwu_set_active_index(active_idx));
 
-   ut_assertok(fwu_get_mdata(dev, ));
+   ut_assertok(fwu_get_mdata());
ut_asserteq(mdata.active_index, active_idx);
 
return 0;
 }
 DM_TEST(dm_test_fwu_mdata_write, UT_TESTF_SCAN_FDT);
-
-static int dm_test_fwu_mdata_check(struct unit_test_state *uts)
-{
-   struct udevice *dev;
-
-   ut_assertok(setup_blk_device(uts));
-   ut_assertok(populate_mmc_disk_image(uts));
-   ut_assertok(write_mmc_blk_device(uts));
-
-   ut_assertok(uclass_first_device_err(UCLASS_FWU_MDATA, ));
-
-   ut_assertok(fwu_check_mdata_validity());
-
-   return 0;
-}
-DM_TEST(dm_test_fwu_mdata_check, UT_TESTF_SCAN_FDT);
-- 
2.34.1



[PATCH v6 5/7] fwu: meta-data: switch to management by common code

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

The common code can now read, verify and fix meta-data copies
while exposing one consistent structure to users.
 Only the .read_mdata() and .write_mdata() callbacks of fwu_mdata_ops
are needed. Get rid of .get_mdata() .update_mdata() .get_mdata_part_num()
.read_mdata_partition() and .write_mdata_partition() and also the
corresponding wrapper functions.

Signed-off-by: Jassi Brar 
Reviewed-by: Etienne Carriere 
---
 cmd/fwu_mdata.c  |  17 +-
 drivers/fwu-mdata/fwu-mdata-uclass.c | 165 ---
 drivers/fwu-mdata/gpt_blk.c  | 124 +-
 include/fwu.h| 199 ---
 lib/fwu_updates/fwu.c| 235 ---
 5 files changed, 38 insertions(+), 702 deletions(-)

diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
index f04af27de6..9b70340368 100644
--- a/cmd/fwu_mdata.c
+++ b/cmd/fwu_mdata.c
@@ -43,23 +43,10 @@ static void print_mdata(struct fwu_mdata *mdata)
 int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,
 int argc, char * const argv[])
 {
-   struct udevice *dev;
int ret = CMD_RET_SUCCESS, res;
-   struct fwu_mdata mdata = { 0 };
+   struct fwu_mdata mdata;
 
-   if (uclass_get_device(UCLASS_FWU_MDATA, 0, ) || !dev) {
-   log_err("Unable to get FWU metadata device\n");
-   return CMD_RET_FAILURE;
-   }
-
-   res = fwu_check_mdata_validity();
-   if (res < 0) {
-   log_err("FWU Metadata check failed\n");
-   ret = CMD_RET_FAILURE;
-   goto out;
-   }
-
-   res = fwu_get_mdata(dev, );
+   res = fwu_get_verified_mdata();
if (res < 0) {
log_err("Unable to get valid FWU metadata\n");
ret = CMD_RET_FAILURE;
diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c 
b/drivers/fwu-mdata/fwu-mdata-uclass.c
index e03773c584..0a8edaaa41 100644
--- a/drivers/fwu-mdata/fwu-mdata-uclass.c
+++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
@@ -14,7 +14,6 @@
 
 #include 
 #include 
-#include 
 
 /**
  * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
@@ -50,170 +49,6 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata 
*mdata, bool primary)
return ops->write_mdata(dev, mdata, primary);
 }
 
-/**
- * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers
- * @dev: FWU metadata device
- * @mdata_parts: array for storing the metadata partition numbers
- *
- * Get the partition numbers on the storage device on which the
- * FWU metadata is stored. Two partition numbers will be returned.
- *
- * Return: 0 if OK, -ve on error
- *
- */
-int fwu_get_mdata_part_num(struct udevice *dev, uint *mdata_parts)
-{
-   const struct fwu_mdata_ops *ops = device_get_ops(dev);
-
-   if (!ops->get_mdata_part_num) {
-   log_debug("get_mdata_part_num() method not defined\n");
-   return -ENOSYS;
-   }
-
-   return ops->get_mdata_part_num(dev, mdata_parts);
-}
-
-/**
- * fwu_read_mdata_partition() - Read the FWU metadata from a partition
- * @dev: FWU metadata device
- * @mdata: Copy of the FWU metadata
- * @part_num: Partition number from which FWU metadata is to be read
- *
- * Read the FWU metadata from the specified partition number
- *
- * Return: 0 if OK, -ve on error
- *
- */
-int fwu_read_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata,
-uint part_num)
-{
-   const struct fwu_mdata_ops *ops = device_get_ops(dev);
-
-   if (!ops->read_mdata_partition) {
-   log_debug("read_mdata_partition() method not defined\n");
-   return -ENOSYS;
-   }
-
-   return ops->read_mdata_partition(dev, mdata, part_num);
-}
-
-/**
- * fwu_write_mdata_partition() - Write the FWU metadata to a partition
- * @dev: FWU metadata device
- * @mdata: Copy of the FWU metadata
- * @part_num: Partition number to which FWU metadata is to be written
- *
- * Write the FWU metadata to the specified partition number
- *
- * Return: 0 if OK, -ve on error
- *
- */
-int fwu_write_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata,
- uint part_num)
-{
-   const struct fwu_mdata_ops *ops = device_get_ops(dev);
-
-   if (!ops->write_mdata_partition) {
-   log_debug("write_mdata_partition() method not defined\n");
-   return -ENOSYS;
-   }
-
-   return ops->write_mdata_partition(dev, mdata, part_num);
-}
-
-/**
- * fwu_mdata_check() - Check if the FWU metadata is valid
- * @dev: FWU metadata device
- *
- * Validate both copies of the FWU metadata. If one of the copies
- * has gone bad, restore it from the other copy.
- *
- * Return: 0 if OK, -ve on error
- *
- */
-int fwu_mdata_check(struct udevice *dev)
-{
-   const struct fwu_mdata_ops *ops = device_get_ops(dev);
-
-   if (!ops->check_mdata) {
-   log_debug("check_mdata() method not 

[PATCH v6 6/7] fwu: rename fwu_get_verified_mdata to fwu_get_mdata

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

fwu_get_mdata() sounds more appropriate than fwu_get_verified_mdata()

Signed-off-by: Jassi Brar 
Reviewed-by: Etienne Carriere 
Reviewed-by: Ilias Apalodimas 
---
 cmd/fwu_mdata.c   | 2 +-
 include/fwu.h | 4 ++--
 lib/fwu_updates/fwu.c | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c
index 9b70340368..5ecda455df 100644
--- a/cmd/fwu_mdata.c
+++ b/cmd/fwu_mdata.c
@@ -46,7 +46,7 @@ int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag,
int ret = CMD_RET_SUCCESS, res;
struct fwu_mdata mdata;
 
-   res = fwu_get_verified_mdata();
+   res = fwu_get_mdata();
if (res < 0) {
log_err("Unable to get valid FWU metadata\n");
ret = CMD_RET_FAILURE;
diff --git a/include/fwu.h b/include/fwu.h
index 314aadea59..33b4d0b3db 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -80,7 +80,7 @@ int fwu_read_mdata(struct udevice *dev, struct fwu_mdata 
*mdata, bool primary);
 int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool 
primary);
 
 /**
- * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
+ * fwu_get_mdata() - Read, verify and return the FWU metadata
  *
  * Read both the metadata copies from the storage media, verify their checksum,
  * and ascertain that both copies match. If one of the copies has gone bad,
@@ -88,7 +88,7 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata 
*mdata, bool primary);
  *
  * Return: 0 if OK, -ve on error
 */
-int fwu_get_verified_mdata(struct fwu_mdata *mdata);
+int fwu_get_mdata(struct fwu_mdata *mdata);
 
 /**
  * fwu_get_active_index() - Get active_index from the FWU metadata
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 49c9316c70..a24ccf567a 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -189,7 +189,7 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata)
 }
 
 /**
- * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
+ * fwu_get_mdata() - Read, verify and return the FWU metadata
  * @mdata: Output FWU metadata read or NULL
  *
  * Read both the metadata copies from the storage media, verify their checksum,
@@ -198,7 +198,7 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata)
  *
  * Return: 0 if OK, -ve on error
  */
-int fwu_get_verified_mdata(struct fwu_mdata *mdata)
+int fwu_get_mdata(struct fwu_mdata *mdata)
 {
int err;
bool parts_ok[2] = { false };
@@ -615,7 +615,7 @@ static int fwu_boottime_checks(void *ctx, struct event 
*event)
return ret;
}
 
-   ret = fwu_get_verified_mdata(NULL);
+   ret = fwu_get_mdata(NULL);
if (ret) {
log_debug("Unable to read meta-data\n");
return ret;
-- 
2.34.1



[PATCH v6 4/7] fwu: gpt: implement read_mdata and write_mdata callbacks

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

Moving towards using common code for meta-data management,
implement the read/write mdata hooks.

Signed-off-by: Jassi Brar 
Reviewed-by: Etienne Carriere 
Reviewed-by: Ilias Apalodimas 
---
 drivers/fwu-mdata/gpt_blk.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
index 28f5d23e1e..be1a6b03a1 100644
--- a/drivers/fwu-mdata/gpt_blk.c
+++ b/drivers/fwu-mdata/gpt_blk.c
@@ -272,7 +272,43 @@ static int fwu_mdata_gpt_blk_probe(struct udevice *dev)
return 0;
 }
 
+static int fwu_gpt_read_mdata(struct udevice *dev, struct fwu_mdata *mdata,
+bool primary)
+{
+   struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
+   struct blk_desc *desc = dev_get_uclass_plat(priv->blk_dev);
+   int ret;
+
+   ret = gpt_get_mdata_partitions(desc);
+   if (ret < 0) {
+   log_debug("Error getting the FWU metadata partitions\n");
+   return -ENOENT;
+   }
+
+   return gpt_read_write_mdata(desc, mdata, MDATA_READ,
+   primary ? g_mdata_part[0] : 
g_mdata_part[1]);
+}
+
+static int fwu_gpt_write_mdata(struct udevice *dev, struct fwu_mdata *mdata,
+bool primary)
+{
+   struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
+   struct blk_desc *desc = dev_get_uclass_plat(priv->blk_dev);
+   int ret;
+
+   ret = gpt_get_mdata_partitions(desc);
+   if (ret < 0) {
+   log_debug("Error getting the FWU metadata partitions\n");
+   return -ENOENT;
+   }
+
+   return gpt_read_write_mdata(desc, mdata, MDATA_WRITE,
+   primary ? g_mdata_part[0] : 
g_mdata_part[1]);
+}
+
 static const struct fwu_mdata_ops fwu_gpt_blk_ops = {
+   .read_mdata = fwu_gpt_read_mdata,
+   .write_mdata = fwu_gpt_write_mdata,
.get_mdata = fwu_gpt_get_mdata,
.update_mdata = fwu_gpt_update_mdata,
.get_mdata_part_num = fwu_gpt_get_mdata_partitions,
-- 
2.34.1



[PATCH v6 3/7] fwu: move meta-data management in core

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

Instead of each i/f having to implement their own meta-data verification
and storage, move the logic in common code. This simplifies the i/f code
much simpler and compact.

Signed-off-by: Jassi Brar 
---
 drivers/fwu-mdata/fwu-mdata-uclass.c |  34 +++
 include/fwu.h|  41 +
 lib/fwu_updates/fwu.c| 131 ++-
 3 files changed, 201 insertions(+), 5 deletions(-)

diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c 
b/drivers/fwu-mdata/fwu-mdata-uclass.c
index b477e9603f..e03773c584 100644
--- a/drivers/fwu-mdata/fwu-mdata-uclass.c
+++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
@@ -16,6 +16,40 @@
 #include 
 #include 
 
+/**
+ * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
+{
+   const struct fwu_mdata_ops *ops = device_get_ops(dev);
+
+   if (!ops->read_mdata) {
+   log_debug("read_mdata() method not defined\n");
+   return -ENOSYS;
+   }
+
+   return ops->read_mdata(dev, mdata, primary);
+}
+
+/**
+ * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata()
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary)
+{
+   const struct fwu_mdata_ops *ops = device_get_ops(dev);
+
+   if (!ops->write_mdata) {
+   log_debug("write_mdata() method not defined\n");
+   return -ENOSYS;
+   }
+
+   return ops->write_mdata(dev, mdata, primary);
+}
+
 /**
  * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers
  * @dev: FWU metadata device
diff --git a/include/fwu.h b/include/fwu.h
index 0919ced812..13f8fdeb28 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -24,6 +24,26 @@ struct fwu_mdata_gpt_blk_priv {
  * @update_mdata() - Update the FWU metadata copy
  */
 struct fwu_mdata_ops {
+   /**
+* read_mdata() - Populate the asked FWU metadata copy
+* @dev: FWU metadata device
+* @mdata: Output FWU mdata read
+* @primary: If primary or secondary copy of metadata is to be read
+*
+* Return: 0 if OK, -ve on error
+*/
+   int (*read_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool 
primary);
+
+   /**
+* write_mdata() - Write the given FWU metadata copy
+* @dev: FWU metadata device
+* @mdata: Copy of the FWU metadata to write
+* @primary: If primary or secondary copy of metadata is to be written
+*
+* Return: 0 if OK, -ve on error
+*/
+   int (*write_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool 
primary);
+
/**
 * check_mdata() - Check if the FWU metadata is valid
 * @dev:FWU device
@@ -126,6 +146,27 @@ struct fwu_mdata_ops {
EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
 0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
 
+/**
+ * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
+ */
+int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary);
+
+/**
+ * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata()
+ */
+int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool 
primary);
+
+/**
+ * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
+ *
+ * Read both the metadata copies from the storage media, verify their checksum,
+ * and ascertain that both copies match. If one of the copies has gone bad,
+ * restore it from the good copy.
+ *
+ * Return: 0 if OK, -ve on error
+*/
+int fwu_get_verified_mdata(struct fwu_mdata *mdata);
+
 /**
  * fwu_check_mdata_validity() - Check for validity of the FWU metadata copies
  *
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index 5313d07302..8f1c05ad1c 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -15,13 +15,13 @@
 #include 
 #include 
 
+#include 
+
+static struct fwu_mdata g_mdata; /* = {0} makes uninit crc32 always invalid */
+static struct udevice *g_dev;
 static u8 in_trial;
 static u8 boottime_check;
 
-#include 
-#include 
-#include 
-
 enum {
IMAGE_ACCEPT_SET = 1,
IMAGE_ACCEPT_CLEAR,
@@ -161,6 +161,127 @@ static int fwu_get_image_type_id(u8 *image_index, 
efi_guid_t *image_type_id)
return -ENOENT;
 }
 
+/**
+ * fwu_sync_mdata() - Update given meta-data partition(s) with the copy 
provided
+ * @mdata: FWU metadata structure
+ * @part: Bitmask of FWU metadata partitions to be written to
+ *
+ * Return: 0 if OK, -ve on error
+ */
+static int fwu_sync_mdata(struct fwu_mdata *mdata, int part)
+{
+   void *buf = >version;
+   int err;
+
+   if (part == BOTH_PARTS) {
+   err = fwu_sync_mdata(mdata, SECONDARY_PART);
+   if (err)
+   return err;
+   part = PRIMARY_PART;
+   }
+
+   /*
+* Calculate the 

[PATCH v6 2/7] fwu: gpt: use cached meta-data partition numbers

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

Use cached values and avoid parsing and scanning through partitions
everytime for meta-data partitions because they can't change after bootup.

Acked-by: Etienne Carriere 
Reviewed-by: Ilias Apalodimas 
Signed-off-by: Jassi Brar 
---
 drivers/fwu-mdata/gpt_blk.c | 43 +
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c
index d35ce49c5c..28f5d23e1e 100644
--- a/drivers/fwu-mdata/gpt_blk.c
+++ b/drivers/fwu-mdata/gpt_blk.c
@@ -24,8 +24,9 @@ enum {
MDATA_WRITE,
 };
 
-static int gpt_get_mdata_partitions(struct blk_desc *desc,
-   uint mdata_parts[2])
+static uint g_mdata_part[2]; /* = {0, 0} to check against uninit parts */
+
+static int gpt_get_mdata_partitions(struct blk_desc *desc)
 {
int i, ret;
u32 nparts;
@@ -33,18 +34,19 @@ static int gpt_get_mdata_partitions(struct blk_desc *desc,
struct disk_partition info;
const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID;
 
+   /* if primary and secondary partitions already found */
+   if (g_mdata_part[0] && g_mdata_part[1])
+   return 0;
+
nparts = 0;
-   for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) {
+   for (i = 1; i < MAX_SEARCH_PARTITIONS && nparts < 2; i++) {
if (part_get_info(desc, i, ))
continue;
uuid_str_to_bin(info.type_guid, part_type_guid.b,
UUID_STR_FORMAT_GUID);
 
-   if (!guidcmp(_mdata_guid, _type_guid)) {
-   if (nparts < 2)
-   mdata_parts[nparts] = i;
-   ++nparts;
-   }
+   if (!guidcmp(_mdata_guid, _type_guid))
+   g_mdata_part[nparts++] = i;
}
 
if (nparts != 2) {
@@ -127,26 +129,25 @@ static int fwu_gpt_update_mdata(struct udevice *dev, 
struct fwu_mdata *mdata)
 {
int ret;
struct blk_desc *desc;
-   uint mdata_parts[2];
struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
 
desc = dev_get_uclass_plat(priv->blk_dev);
 
-   ret = gpt_get_mdata_partitions(desc, mdata_parts);
+   ret = gpt_get_mdata_partitions(desc);
if (ret < 0) {
log_debug("Error getting the FWU metadata partitions\n");
return -ENOENT;
}
 
/* First write the primary partition */
-   ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, mdata_parts[0]);
+   ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, g_mdata_part[0]);
if (ret < 0) {
log_debug("Updating primary FWU metadata partition failed\n");
return ret;
}
 
/* And now the replica */
-   ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, mdata_parts[1]);
+   ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, g_mdata_part[1]);
if (ret < 0) {
log_debug("Updating secondary FWU metadata partition failed\n");
return ret;
@@ -158,16 +159,14 @@ static int fwu_gpt_update_mdata(struct udevice *dev, 
struct fwu_mdata *mdata)
 static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata *mdata)
 {
int ret;
-   uint mdata_parts[2];
-
-   ret = gpt_get_mdata_partitions(desc, mdata_parts);
 
+   ret = gpt_get_mdata_partitions(desc);
if (ret < 0) {
log_debug("Error getting the FWU metadata partitions\n");
return -ENOENT;
}
 
-   ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, mdata_parts[0]);
+   ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, g_mdata_part[0]);
if (ret < 0) {
log_debug("Failed to read the FWU metadata from the device\n");
return -EIO;
@@ -182,7 +181,7 @@ static int gpt_get_mdata(struct blk_desc *desc, struct 
fwu_mdata *mdata)
 * Try to read the replica.
 */
memset(mdata, '\0', sizeof(struct fwu_mdata));
-   ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, mdata_parts[1]);
+   ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, g_mdata_part[1]);
if (ret < 0) {
log_debug("Failed to read the FWU metadata from the device\n");
return -EIO;
@@ -206,9 +205,15 @@ static int fwu_gpt_get_mdata(struct udevice *dev, struct 
fwu_mdata *mdata)
 static int fwu_gpt_get_mdata_partitions(struct udevice *dev, uint *mdata_parts)
 {
struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
+   int err;
+
+   err = gpt_get_mdata_partitions(dev_get_uclass_plat(priv->blk_dev));
+   if (!err) {
+   mdata_parts[0] = g_mdata_part[0];
+   mdata_parts[1] = g_mdata_part[1];
+   }
 
-   return gpt_get_mdata_partitions(dev_get_uclass_plat(priv->blk_dev),
-   mdata_parts);
+   return err;
 

[PATCH v6 1/7] dt/bindings: fwu-mdata-mtd: drop changes outside FWU

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

Any requirement of FWU should not require changes to bindings
of other subsystems. For example, for mtd-backed storage we
can do without requiring 'fixed-partitions' children to also
carry 'uuid', a property which is non-standard and not in the
bindings.

 There exists no code yet, so we can change the fwu-mtd bindings
to contain all properties within the fwu-mdata node.

Signed-off-by: Jassi Brar 
---
 .../firmware/fwu-mdata-mtd.yaml   | 105 +++---
 1 file changed, 91 insertions(+), 14 deletions(-)

diff --git a/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml 
b/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml
index 4f5404f999..6a22aeea30 100644
--- a/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml
+++ b/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml
@@ -1,13 +1,13 @@
 # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/firmware/u-boot,fwu-mdata-sf.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
+$id: http://devicetree.org/schemas/firmware/u-boot,fwu-mdata-mtd.yaml#
+$schema: http://devicetree.org/meta-schemas/base.yaml#
 
 title: FWU metadata on MTD device without GPT
 
 maintainers:
- - Masami Hiramatsu 
+ - Jassi Brar 
 
 properties:
   compatible:
@@ -15,24 +15,101 @@ properties:
   - const: u-boot,fwu-mdata-mtd
 
   fwu-mdata-store:
-maxItems: 1
-description: Phandle of the MTD device which contains the FWU medatata.
+$ref: /schemas/types.yaml#/definitions/phandle
+description: Phandle of the MTD device which contains the FWU MetaData and 
Banks.
 
-  mdata-offsets:
+  mdata-parts:
+$ref: /schemas/types.yaml#/definitions/non-unique-string-array
 minItems: 2
-description: Offsets of the primary and secondary FWU metadata in the NOR 
flash.
+maxItems: 2
+description: labels of the primary and secondary FWU metadata partitions 
in the 'fixed-partitions' subnode of the 'jedec,spi-nor' flash device node.
+
+  patternProperties:
+"fwu-bank[0-9]":
+type: object
+description: List of FWU mtd-backed banks. Typically two banks.
+
+properties:
+  id:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Index of the bank.
+
+  label:
+$ref: /schemas/types.yaml#/definitions/non-unique-string-array
+minItems: 1
+maxItems: 1
+description: label of the partition, in the 'fixed-partitions' subnode 
of the 'jedec,spi-nor' flash device node, that holds this bank.
+
+  patternProperties:
+"fwu-image[0-9]":
+type: object
+description: List of images in the FWU mtd-backed bank.
+
+properties:
+  id:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Index of the bank.
+
+  offset:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Offset, from start of the bank, where the image is 
located.
+
+  size:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: Size reserved for the image.
+
+  uuid:
+$ref: /schemas/types.yaml#/definitions/non-unique-string-array
+minItems: 1
+maxItems: 1
+description: UUID of the image.
+
+required:
+  - id
+  - offset
+  - size
+  - uuid
+additionalProperties: false
+
+required:
+  - id
+  - label
+  - fwu-images
+additionalProperties: false
 
 required:
   - compatible
   - fwu-mdata-store
-  - mdata-offsets
-
+  - mdata-parts
+  - fwu-banks
 additionalProperties: false
 
 examples:
   - |
-fwu-mdata {
-compatible = "u-boot,fwu-mdata-mtd";
-fwu-mdata-store = <>;
-mdata-offsets = <0x50 0x53>;
-};
+   fwu-mdata {
+   compatible = "u-boot,fwu-mdata-mtd";
+   fwu-mdata-store = <>;
+   mdata-parts = "MDATA-Pri", "MDATA-Sec";
+
+   fwu-bank0 {
+   id = <0>;
+   label = "FIP-Bank0";
+   fwu-image0 {
+   id = <0>;
+   offset = <0x0>;
+   size = <0x40>;
+   uuid = "5a66a702-99fd-4fef-a392-c26e261a2828";
+   };
+   };
+   fwu-bank1 {
+   id = <1>;
+   label = "FIP-Bank1";
+   fwu-image0 {
+   id = <0>;
+   offset = <0x0>;
+   size = <0x40>;
+   uuid = "a8f868a1-6e5c-4757-878d-ce63375ef2c0";
+   };
+   };
+   };
+...
-- 
2.34.1



[PATCH v6 0/7] fwu: gpt: implement read_mdata and write_mdata callbacks

2023-03-06 Thread jassisinghbrar
From: Jassi Brar 

The patchset reduces ~400 lines of code, while keeping the functionality same 
and making
meta-data operations much faster (by using cached structures).

Issue:
 meta-data copies (primary and secondary) are being handled by the 
backend/storage layer
instead of the common core in fwu.c (as also noted by Ilias)  that is, 
gpt_blk.c manages
meta-data and similarly raw_mtd.c will have to do the same when it arrives. The 
code
could by make smaller, cleaner and optimised.

Basic idea:
 Introduce  .read_mdata() and .write_mdata() in fwu_mdata_ops  that simply 
read/write
meta-data copy. The core code takes care of integrity and redundancy of the 
meta-data,
as a result we can get rid of every other callback .get_mdata() .update_mdata()
.get_mdata_part_num()  .read_mdata_partition()  .write_mdata_partition() and the
corresponding wrapper functions thereby making the code 100s of LOC smaller.

Get rid of fwu_check_mdata_validity() and fwu_mdata_check() which expected 
underlying
layer to manage and verify mdata copies.
Implement  fwu_get_verified_mdata(struct fwu_mdata *mdata) public function that 
reads,
verifies and, if needed, fixes the meta-data copies.

Verified copy of meta-data is now cached as 'g_mdata' in fwu.c, which avoids 
multiple
low-level expensive read and parse calls.
gpt meta-data partition numbers are now cached in gpt_blk.c, so that we don't 
have to do expensive part_get_info() and uid ops.

Changes since v5:
* Fix SANDBOX tests
* Removed '@' from dt nodes
* misc cosmetic changes suggested by Etienne

Changes since v4:
* Change fwu-mdata-mtd bindings to not require external changes
* Handle 'part == BOTH_PARTS' in fwu_sync_mdata
* use parts_ok[] and parts_mdata[] instead of pri/sec_ok and p/s_mdata

Changes since v3:
* Fix error log wording
* call fwu_write_mdata() with part & PRIMARY_PART ? true: false

Changes since v2:
* Drop whitespace changes
* Fix missing mdata copy before return

Changes since v1:
* Fix typos and misc cosmetic changes
* Catch error returns

Jassi Brar (7):
  dt/bindings: fwu-mdata-mtd: drop changes outside FWU
  fwu: gpt: use cached meta-data partition numbers
  fwu: move meta-data management in core
  fwu: gpt: implement read_mdata and write_mdata callbacks
  fwu: meta-data: switch to management by common code
  fwu: rename fwu_get_verified_mdata to fwu_get_mdata
  test: dm: fwu: fix for the updated api

 cmd/fwu_mdata.c   |  17 +-
 .../firmware/fwu-mdata-mtd.yaml   | 105 ++-
 drivers/fwu-mdata/fwu-mdata-uclass.c  | 151 +
 drivers/fwu-mdata/gpt_blk.c   | 175 +++
 include/fwu.h | 198 ++--
 lib/fwu_updates/fwu.c | 296 --
 test/dm/fwu_mdata.c   |  22 +-
 7 files changed, 299 insertions(+), 665 deletions(-)

-- 
2.34.1



Re: [PATCH v6 12/22] core: fdtaddr: add devfdt_get_addr_size_index_ptr function

2023-03-06 Thread Johan Jonker



On 3/6/23 18:53, Simon Glass wrote:
> Hi Johan,
> 
> On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
>>
>> Add devfdt_get_addr_size_index_ptr function with the same
>> functionality as devfdt_get_addr_size_index, but instead
>> a return pointer is given.
>>
>> Suggested-by: Michael Nazzareno Trimarchi 
>> Signed-off-by: Johan Jonker 
>> Reviewed-by: Michael Trimarchi 
>> ---
>>
>> Changed V5:
>>   fix spelling
>>   use tabs
>> ---
>>  drivers/core/fdtaddr.c |  8 
>>  include/dm/fdtaddr.h   | 17 -
>>  2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
>> index 91bcd1a2..f5906ff9 100644
>> --- a/drivers/core/fdtaddr.c
>> +++ b/drivers/core/fdtaddr.c
>> @@ -122,6 +122,14 @@ fdt_addr_t devfdt_get_addr_size_index(const struct 
>> udevice *dev, int index,
>>  #endif
>>  }
>>
>> +void *devfdt_get_addr_size_index_ptr(const struct udevice *dev, int index,
>> +fdt_size_t *size)
>> +{
>> +   fdt_addr_t addr = devfdt_get_addr_size_index(dev, index, size);
>> +
>> +   return (addr == FDT_ADDR_T_NONE) ? NULL : (void *)(uintptr_t)addr;
> 
Just wondering, as a side question:
Why is Uboot maintaining/exporting 2 sets of functions that do the do the more 
or less the same thing.

For example:
devfdt_get_addr_size_index_ptr vs. dev_read_addr_size_index_ptr

Or should we standardize and replace all by dev_read_addr_size_index_ptr if 
possible?

Johan

> 
> [..]
> 
> Regards,
> SImon


Re: [PATCH v5 3/6] fwu: move meta-data management in core

2023-03-06 Thread Jassi Brar
On Wed, Mar 1, 2023 at 5:16 AM Etienne Carriere
 wrote:
> On Tue, 28 Feb 2023 at 01:52,  wrote:
> >
> > From: Jassi Brar 
> >
> > Instead of each i/f having to implement their own meta-data verification
> > and storage, move the logic in common code. This simplifies the i/f code
> > much simpler and compact.
> >
...

> > diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c 
> > b/drivers/fwu-mdata/fwu-mdata-uclass.c
> > index b477e9603f..e03773c584 100644
> > --- a/drivers/fwu-mdata/fwu-mdata-uclass.c
> > +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c
> > @@ -16,6 +16,40 @@
> >  #include 
> >  #include 
> >
> > +/**
> > + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata()
> > + *
> > + * Return: 0 if OK, -ve on error
> > + */
>
> Inline description only in header file, or duplicated in source and
> header files?
>
This is the original practice in FWU stack - to have the description
in header as well as source code. I just didn't want to stick out.


> > diff --git a/include/fwu.h b/include/fwu.h
> > index 0919ced812..1a700c9e6a 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -24,6 +24,26 @@ struct fwu_mdata_gpt_blk_priv {
> >   * @update_mdata() - Update the FWU metadata copy
> >   */
> >  struct fwu_mdata_ops {
> > +   /**
> > +* read_mdata() - Populate the asked FWU metadata copy
> > +* @dev: FWU metadata device
> > +* @mdata: Copy of the FWU metadata
>
> @mdata: Output FWU mdata read
>
> > +* @primary: If primary or secondary copy of meta-data is to be read
>
> s/meta-data/FWU metadata/
> Ditto in .write_mdata description
>
ok

> > +/**
> > + * fwu_get_verified_mdata() - Read, verify and return the FWU metadata
> > + *
> > + * Read both the metadata copies from the storage media, verify their 
> > checksum,
> > + * and ascertain that both copies match. If one of the copies has gone bad,
> > + * restore it from the good copy.
>
> @mdata: Output FWU metadata read or NULL
>
ok

> > +static int fwu_sync_mdata(struct fwu_mdata *mdata, int part)
> > +{
> > +   void *buf = >version;
> > +   int err;
> > +
> > +   if (part == BOTH_PARTS) {
> > +   err = fwu_sync_mdata(mdata, SECONDARY_PART);
> > +   if (err)
> > +   return err;
> > +   part = PRIMARY_PART;
> > +   }
> > +
> > +   /*
> > +* Calculate the crc32 for the updated FWU metadata
> > +* and put the updated value in the FWU metadata crc32
> > +* field
> > +*/
> > +   mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32));
> > +
> > +   err = fwu_write_mdata(g_dev, mdata, part & PRIMARY_PART ? true : 
> > false);
>
> Since this expects part is either PRIMARY_PART or SECONDARY_PART, prefer:
>  err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART);
>
> And ditto below:
>   part == PRIMARY_PART ? "primary": "secondary");
>
yes, now that we handle out the BOTH_PARTS case already.


> > +int fwu_get_verified_mdata(struct fwu_mdata *mdata)
> > +{
> > +   int err;
> > +   bool parts_ok[2] = { false };
> > +   struct fwu_mdata s, *parts_mdata[2];
> > +
> > +   parts_mdata[0] = _mdata;
> > +   parts_mdata[1] = 
> > +
> > +   /* if mdata already read and ready */
> > +   err = mdata_crc_check(parts_mdata[0]);
> > +   if (!err)
> > +   goto ret_mdata;
> > +   /* else read, verify and, if needed, fix mdata */
> > +
> > +   for (int i = 0; i < 2; i++) {
>
> Define "int i;" at function block entry?
>
Hmm... I prefer this way - limiting scope of the scratch variables.

thanks.


Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

2023-03-06 Thread Johan Jonker



On 3/6/23 19:20, Simon Glass wrote:
> Hi Johan,
> 
> On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
>>
>> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
>> can expect 64-bit data from the device tree parser, so use
> 
> Why is that? It seems quite inefficient.
1:
===
Because the device tree does describes more then just only the internal io/mem 
range.
When a NAND chip is connected it must be able to describe partitions far beyond 
that 32bit range.
This change only changes the PARSE capacity defined by fdt_addr_t and 
fdt_size_t and not the phys_addr_t and phys_size_t.
Most drivers make a little mess when taking a DT reg value and then trying to 
make it fit in a structure of multiple registers with various offsets.
Fixing all of that is beyond my capacity/this serie and more a MAINTAINER task.

https://elixir.bootlin.com/u-boot/latest/source/drivers/mtd/mtdpart.c#L933

struct mtd_partition {
const char *name;   /* identifier string */
uint64_t size;  /* partition size */
uint64_t offset;/* offset within the master MTD space */
uint32_t mask_flags;/* master MTD flags to mask out for 
this partition */
struct nand_ecclayout *ecclayout;   /* out of band layout for this 
partition (NAND only) */
};

int add_mtd_partitions_of(struct mtd_info *master)
struct mtd_partition part = { 0 };
fdt_addr_t offset;
fdt_size_t size;

[..]
part.offset = offset;
part.size = size;

While the mtd_partition structure is ready with uint64_t size all the parse 
functions that export this reg value were typedef to a 32bit value.

===
Given mk808 rk3066a with NAND:

[   38.750789] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit
[   38.756650] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, OOB 
size: 640

===
BEFORE:

List of MTD devices:
* nand0
  - type: MLC NAND flash
  - block size: 0x20 bytes
  - min I/O: 0x2000 bytes
  - OOB size: 640 bytes
  - OOB available: 4294967290 bytes
  - ECC strength: 40 bits
  - ECC step size: 1024 bytes
  - bitflip threshold: 30 bits
  - 0x-0x0002 : "nand0"
  - 0x0040-0x0060 : "boot-blk-0"
  - 0x0060-0x0080 : "boot-blk-1"
  - 0x0080-0x00a0 : "boot-blk-2"
  - 0x00a0-0x00c0 : "boot-blk-3"
  - 0x00c0-0x00e0 : "boot-blk-4"
  - 0x0100-0xfe00 : "rootfs"
  - 0xfe00-0x0001 : "bbt"

# This output is corrupted.

===
AFTER:

List of MTD devices:
* nand0
  - type: MLC NAND flash
  - block size: 0x20 bytes
  - min I/O: 0x2000 bytes
  - OOB size: 640 bytes
  - OOB available: 4294967290 bytes
  - ECC strength: 40 bits
  - ECC step size: 1024 bytes
  - bitflip threshold: 30 bits
  - 0x-0x0002 : "nand0"
  - 0x0040-0x0060 : "boot-blk-0"
  - 0x0060-0x0080 : "boot-blk-1"
  - 0x0080-0x00a0 : "boot-blk-2"
  - 0x00a0-0x00c0 : "boot-blk-3"
  - 0x00c0-0x00e0 : "boot-blk-4"
  - 0x0100-0x0001fe00 : "rootfs"
  - 0x0001fe00-0x0002 : "bbt"
===
Example:
partitions {
compatible = "fixed-partitions";
#address-cells = <2>;
#size-cells = <2>;

partition@40 {
reg = <0x0 0x0040 0x0 0x0020>;
label = "boot-blk-0";
};

partition@60 {
reg = <0x0 0x0060 0x0 0x0020>;
label = "boot-blk-1";
};

partition@80 {
reg = <0x0 0x0080 0x0 0x0020>;
label = "boot-blk-2";
};

partition@a0 {
reg = <0x0 0x00a0 0x0 0x0020>;
label = "boot-blk-3";
};

partition@c0 {
reg = <0x0 0x00c0 0x0 0x0020>;
label = "boot-blk-4";
};

partition@100 {
reg = <0x0 0x0100 0x1 0xfd00>;
label = "rootfs";
};

partition@1fe00 {
reg = <0x1 0xfe00 0x0 0x0200>;
label = "bbt";
};
};
===

2:
As a side effect Rockchip rk3288 with 32bit reg should be 

Re: Please pull u-boot-marvell/master

2023-03-06 Thread Tom Rini
On Mon, Mar 06, 2023 at 12:52:52PM +0100, Stefan Roese wrote:

> Hi Tom,
> 
> please pull this small Marvell related fix:
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] arm: mach-k3: introduce generic board detction kconfig option

2023-03-06 Thread Tom Rini
On Fri, Mar 03, 2023 at 08:16:28PM +0100, Christian Gmeiner wrote:

> For non TI boards it is not possible to enable the do_board_detect()
> call as TI_I2C_BOARD_DETECT is defined in board/ti/common/Kconfig.
> 
> I want to use do_board_detect() to dectect boards and properties based
> on some SPI communication with a FPGA.
> 
> Signed-off-by: Christian Gmeiner 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 18/19] test/py: android: extend abootimg test

2023-03-06 Thread Tom Rini
On Mon, Mar 06, 2023 at 08:49:02PM +0100, Safae Ouajih wrote:
> 
> On 27/02/2023 15:18, Tom Rini wrote:
> > On Mon, Feb 27, 2023 at 03:15:31PM +0100, Safae Ouajih wrote:
> > > On 07/02/2023 20:02, Tom Rini wrote:
> > > > On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih wrote:
> > > > 
> > > > > test_abootimg is extended to include the testing of boot images
> > > > > version 4. For this, boot.img and vendor_boot.img have been
> > > > > generated using mkbootimg tool with setting the header
> > > > > version to 4.
> > > > > 
> > > > > This tests:
> > > > > - Getting the header version using abootimg
> > > > > - Extracting the load address of the dtb
> > > > > - Extracting the dtb start address in RAM
> > > > > 
> > > > > Running test:
> > > > > $ ./test/py/test.py --bd sandbox --build -k test_abootimg
> > > > > 
> > > > > Signed-off-by: Safae Ouajih 
> > > > > Reviewed-by: Simon Glass 
> > > > > ---
> > > > >test/py/tests/test_android/test_abootimg.py | 136 
> > > > > ++--
> > > > >1 file changed, 123 insertions(+), 13 deletions(-)
> > > > Alright, so I don't know where the failure starts, exactly. And to make
> > > > testing this easier, there's currently the
> > > > trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can
> > > > use to replicate my problem. The problem is that while this test passes
> > > > in CI, with GCC, with Clang it fails, consistently:
> > > > https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284
> > > > and I'm not quite sure why. I hope that building sandbox with clang and
> > > > also just trying these features out interactively will fail too, and so
> > > > debugging this will be less of a problem.
> > > > 
> > > Hello Tom,
> > > 
> > > I have investigated this issue, clang has a strange behavior in:
> > > 
> > > * abootimg_get_dtb_load_addr() : cmd/abootimg.c
> > > * android_image_get_dtb_by_index() : boot/image-android.c
> > > 
> > > That is probably linked to some sort of optimization clang does.
> > > 
> > > However, The fail is not reproducible using clang-15 and clang-16 and also
> > > not reproducible when turning off clang optimizations.
> > > 
> > > I suggest using clang-15 to run the test or I can remove all optimizations
> > > 
> > > on the related functions if clang-14 is used.
> > Thanks for investigating.  I see that 15 is now considered stable, so
> > I'll update the next branch for that, then re-take this series.
> > 
> 
> Hello Tom,
> 
> Thank you.
> 
> I am a bit confused, do you mean that you will apply this series after
> clang-15 update?

Yes, and the series to move CI to clang-15 is currently stuck on some
pytest (real) failures that need to be resolved. But I would expect this
to all be resolved in time for this series here to be included in
v2023.07.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 18/19] test/py: android: extend abootimg test

2023-03-06 Thread Safae Ouajih



On 27/02/2023 15:18, Tom Rini wrote:

On Mon, Feb 27, 2023 at 03:15:31PM +0100, Safae Ouajih wrote:

On 07/02/2023 20:02, Tom Rini wrote:

On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih wrote:


test_abootimg is extended to include the testing of boot images
version 4. For this, boot.img and vendor_boot.img have been
generated using mkbootimg tool with setting the header
version to 4.

This tests:
- Getting the header version using abootimg
- Extracting the load address of the dtb
- Extracting the dtb start address in RAM

Running test:
$ ./test/py/test.py --bd sandbox --build -k test_abootimg

Signed-off-by: Safae Ouajih 
Reviewed-by: Simon Glass 
---
   test/py/tests/test_android/test_abootimg.py | 136 ++--
   1 file changed, 123 insertions(+), 13 deletions(-)

Alright, so I don't know where the failure starts, exactly. And to make
testing this easier, there's currently the
trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can
use to replicate my problem. The problem is that while this test passes
in CI, with GCC, with Clang it fails, consistently:
https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284
and I'm not quite sure why. I hope that building sandbox with clang and
also just trying these features out interactively will fail too, and so
debugging this will be less of a problem.


Hello Tom,

I have investigated this issue, clang has a strange behavior in:

* abootimg_get_dtb_load_addr() : cmd/abootimg.c
* android_image_get_dtb_by_index() : boot/image-android.c

That is probably linked to some sort of optimization clang does.

However, The fail is not reproducible using clang-15 and clang-16 and also
not reproducible when turning off clang optimizations.

I suggest using clang-15 to run the test or I can remove all optimizations

on the related functions if clang-14 is used.

Thanks for investigating.  I see that 15 is now considered stable, so
I'll update the next branch for that, then re-take this series.



Hello Tom,

Thank you.

I am a bit confused, do you mean that you will apply this series after 
clang-15 update?


Best regards,

Safae



Re: [PATCH] cmd: fdt: Drop the 0x prefix

2023-03-06 Thread Marek Vasut

On 3/6/23 18:53, Simon Glass wrote:

Hi Marek,

On Wed, 1 Mar 2023 at 20:04, Marek Vasut
 wrote:


The 'fdt get addr' is always assumed to be hex value, drop the prefix.
Since this might break existing users who depend on the existing
behavior with 0x prefix, this is a separate patch. Revert if this
breaks anything.

Signed-off-by: Marek Vasut 
---
Cc: Heinrich Schuchardt 
Cc: Simon Glass 
Cc: Tom Rini 
---
  cmd/fdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/fdt.c b/cmd/fdt.c
index f38fe909c3e..04b664e652c 100644
--- a/cmd/fdt.c
+++ b/cmd/fdt.c
@@ -478,7 +478,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
 /* Get address */
 char buf[19];

-   snprintf(buf, sizeof(buf), "0x%lx",
+   snprintf(buf, sizeof(buf), "%lx",
  (ulong)map_to_sysmem(nodep));
 env_set(var, buf);
 } else if (subcmd[0] == 's') {
--
2.39.2



iwc how about using env_sethex() ?


The 'env get size' 's' case below could likely use similar treatment , 
do I read it right ?


Re: [PATCH v4 3/4] Kconfig: j721e: Change K3_MCU_SCRATCHPAD_BASE to non firewalled region

2023-03-06 Thread Tom Rini
On Mon, Mar 06, 2023 at 11:12:53AM +0530, Manorit Chawdhry wrote:
> In non-combined boot flow for K3, all the firewalls are locked by default
> until sysfw comes up. Rom configures some of the firewall for its usage
> along with the SRAM for R5 but the PSRAM region is still locked.
> 
> The K3 MCU Scratchpad for j721e was set to a PSRAM region triggering the
> firewall exception before sysfw came up. The exception started happening
> after adding multi dtb support that accesses the scratchpad for reading
> EEPROM contents.
> 
> The commit changes R5 MCU scratchpad for j721e to an SRAM region.
> 
> Old Map:
> ┌─┐ 0x41c0
> │ SPL │
> ├─┤ 0x41c4 (approx)
> │STACK│
> ├─┤ 0x41c85b20
> │ Global data │
> │  sizeof(struct global_data) = 0xd8  │
> ├─┤ gd->malloc_base = 0x41c85bfc
> │HEAP │
> │  CONFIG_SYS_MALLOC_F_LEN = 0x7  │
> ├─┤ CONFIG_SPL_BSS_START_ADDR
> │   SPL BSS   │ (0x41cf5bfc)
> │  CONFIG_SPL_BSS_MAX_SIZE = 0xA000   │
> └─┘ CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX
>   (0x41cffbfc)
> 
> New Map:
> ┌─┐ 0x41c0
> │ SPL │
> ├─┤ 0x41c4 (approx)
> │EMPTY│
> ├─┤ 0x41c81920
> │STACK│
> │ SPL_SIZE_LIMIT_PROVIDE_STACK=0x4000 │
> ├─┤ 0x41c85920
> │ Global data │
> │  sizeof(struct global_data) = 0xd8  │
> ├─┤ gd->malloc_base = 0x41c859f0
> │HEAP │
> │  CONFIG_SYS_MALLOC_F_LEN = 0x7  │
> ├─┤ CONFIG_SPL_BSS_START_ADDR
> │   SPL BSS   │ (0x41cf59f0)
> │  CONFIG_SPL_BSS_MAX_SIZE = 0xA000   │
> ├─┤ 0x41cff9fc
> │ NEW MCU SCRATCHPAD  │
> │  SYS_K3_MCU_SCRATCHPAD_SIZE = 0x200 │
> └─┘ CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX
>   (0x41cffbfc)
> 
> Fixes: ab977c8b91b4 ("configs: j721s2_evm_r5: Enable support for building 
> multiple dtbs into FIT")
> 
> Signed-off-by: Manorit Chawdhry 
> [n-fran...@ti.com: SRAM allocation addressing diagram]
> Signed-off-by: Neha Francis 
> Reviewed-by: Tom Rini 
> Reviewed-by: Kamlesh Gurudasani 
> ---
>  arch/arm/mach-k3/Kconfig   |  3 ++-
>  configs/j721e_evm_r5_defconfig | 10 --
>  doc/board/ti/j721e_evm.rst | 27 +++

OK, but now this just renders differently poorly.  Please see the
list-table directive as used for example in doc/board/apple/m1.rst and
it would be good to get other ascii tables updated to produce nice
output as well.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v5 00/44] More tidy-ups of Kconfig options

2023-03-06 Thread Simon Glass
Hi Tom,

On Fri, 3 Mar 2023 at 16:43, Tom Rini  wrote:
>
> On Wed, Feb 22, 2023 at 09:33:41AM -0700, Simon Glass wrote:
>
> > This series was split out of the old 'split config' splc series. It
> > contains clean-up patches which do not depend on split config.
> >
> > This is available at u-boot-dm/spld-working
> >
> > The size changes look pretty good: https://paste.debian.net/1271742/
> >
> > The remaining patches will move into a new 'splg' series (G for Good).
> >
> > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=341504=*
> >
> > Changes in v5:
> > - Fix reply typo
> > - Change approach and expand notes after more investigation
> > - Drop FSL_ISBC_KEY_EXT patch as it changes the size
> > - Drop PHY_CADENCE_SIERRA patch as it changes the size
> >
> > Changes in v4:
> > - Avoid use of def_bool
> > - Modify to get rid of def_bool
> > - Adjust Kconfig ordering
> > - Just fix the typo
> > - Reduce and rename commit
> > - Reduce and rename commit
> > - Fix 'wanderboard' typo
> > - Reduce and rename commit
> >
> > Changes in v3:
> > - Add new patch to disable QFW bootmeth in SPL
> > - Move the option down to the non-SPL part of drivers/Makefile
> > - Correct 'VPL' typo
> > - Use a consistent format for the comment
> > - Fix a transitory build error with sandbox_spl
> > - Add a new patch to disallow commands in SPL
> >
> > Changes in v2:
> > - Rebase to previous series
> >
> > Simon Glass (44):
> >   mtd: Drop unused kb9202_nand driver
> >   mtd: Drop unused CONFIG_ONENAND_U_BOOT
> >   sh4: Drop unused twl6030 driver
> >   moveconfig: Update to detect / correct missing SPL Kconfigs
> >   bootstd: Disable QFW bootmeth in SPL
> >   Correct SPL uses of ARCH_MVEBU
> >   Correct SPL uses of DISPLAY_AER_FULL
> >   Correct SPL uses of MULTIPLEXER
> >   Correct SPL use of PG_WCOM_UBOOT_UPDATE_SUPPORTED
> >   Correct SPL uses of PHY_FIXED
> >   boot: Add Kconfigs for BOOTMETH_VBE_REQUEST
> >   Correct SPL use of DM_RNG
> >   lib: Add a Kconfig for SPL_BZIP2
> >   moveconfig: Various minor improvements
> >   sandbox: Expand size for VPL image
> >   event: Add Kconfig options for SPL
> >   bootstd: Correct 'VPL' typo
> >   env: Avoid checking ENV_IS_IN when env disabled
> >   env: Allow VPL environment to be nowhere
> >   lib: Add VPL options for SHA1 and SHA256
> >   x86: Use string functions for all 32-bit builds
> >   lib: Fix build condition for tiny-printf
> >   sandbox: Tidy up RTC options
> >   sandbox: Use the generic VPL option to enable VPL
> >   sandbox: Tidy up I2C options
> >   fixdep: Add support for VPL
> >   fixdep: Refactor to make testing easier
> >   fixdep: Add some tests for parse_config_line()
> >   test: Add SPL versions of the TEST_KCONFIG options
> >   lib: Add an SPL config for LIB_UUID
> >   test: Tidy up sandbox handling in test-main
> >   x86: Fix up use of X86_32BIT_INIT and X86_64 options
> >   Add VPL options for BLOBLIST
> >   rockchip: Avoid checking environment without ENV_SUPPORT
> >   freescale: Drop old pre-DM_ETH code
> >   imx: Use SATA instead of CMD_SATA
> >   net: Add an SPL config for atheros
> >   freescale: Fix odd use of ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
> >   serial: Support ns16550 driver in TPL
> >   dm: Add a TPL symbol for simple-bus
> >   x86: coral: Add missing TPL options
> >   power: wandboard: Add a missing CONFIG
> >   venice: Simplify conditions for network init
> >   command: Don't allow commands in SPL
>
> I've applied almost all of this series. I've left the moveconfig.py
> stuff for now as I'm not sure it's useful until split config. I've
> followed up about the X86_32BIT_INIT patch as I do want to see that
> boot-tested, and I've left the ns16650 TPL patch as both that feel like
> "split config makes this an issue" rather than an issue we have today
> and fwiw, the dependencies are wrong in that there's no TPL for omap2
> which I didn't reply directly to, I only noticed as reviewing the series
> locally before merging.

OK, thanks for that. Do you think it is worth picking up the fixdep
ones also? The VPL thing is a bug and the others are for tests.

I will see if I can get some time by next week to do another spin. How
can we get more eyes on the Kconfig-language proposal?

Regards,
Simon


Re: [PATCH RFC u-boot-mvebu 0/2] arm: mvebu: Fix eMMC boot

2023-03-06 Thread Pali Rohár
On Monday 06 March 2023 11:15:35 Martin Rowe wrote:
> On Sun, 5 Mar 2023 at 16:04, Pali Rohár  wrote:
> > Could you try another test by completely erasing BOOT0, BOOT1 and USER
> > > data? And see what BootROM prints.
> >
> 
> =
> BootROM - 1.73
> 
> Booting from MMC
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad header at offset 0020
> Switching BootPartitions.
> BootROM: Bad header at offset 
> BootROM: Bad h
> Trying Uart
> =

Originally I though that I did not understand disassembled bootrom code
correctly but this logs proves that I did it correctly. Log is very
strange.

There is a loop which tries partition numbers 0x1, 0x2, ... 0x9, 0xa.

And if I'm looking at the bootrom code correctly it does bit-AND
operation on partition number with constant 0x7 and result is set into
mmc register 179 (partition_config).

So if I understand it correctly it means that bootrom automatically
clears boot_ack and boot_partition. And into partition_access it sets
the partition number. Hence numbers 0x9 and 0xa are trimmed and
aliased to 0x1 and 0x2; and number 0x8 overflows to 0x0.

Completely strange behavior and probably against how HW mmc boot
partitions should be used.

eMMC spec defines:
partition_access (low 3 bits of mmc 179/partition_config register):
0x0 : No access to boot partition (default)
0x1 : R/W boot partition 1
0x2 : R/W boot partition 2
0x3 : R/W Replay Protected Memory Block (RPMB)
0x4 : Access to General Purpose partition 1
0x5 : Access to General Purpose partition 2
0x6 : Access to General Purpose partition 3
0x7 : Access to General Purpose partition 4

I guess that you do not have general purpose partitions layout on emmc
and RPMB is not used too. So technically only 0x0, 0x1, and 0x2 are
available.

To confirm my theory, could you try to do following tests?

1. Check u-boot's "mmc partconf" settings are not preserved across
reboots.

2. Put valid image into userdata partition; erase boot 0 and boot 1; and
post bootrom output. There should be 7 invalid attempts with Switching
BootPartitions message.

3. Take valid image, invalidate kwb header checksum and put it on boot0;
plus erase boot1 and user. Bootrom should print "Invalid header
checksum" message and it should be two times. Once for 0x1 and second
time for overflowed 0x9.


Re: [PATCH v6 13/22] core: read: add dev_read_addr_index_ptr function

2023-03-06 Thread Simon Glass
On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
>
> Add dev_read_addr_index_ptr function with the
> same functionality as dev_read_addr_index,
> but instead a return pointer is given.
> Use map_sysmem() function as cast for the return.
> Make same fix for dev_read_addr_ptr() function.
>
> Signed-off-by: Johan Jonker 
> ---
>
> Changed V6:
>   use map_sysmem()
>
> Changed V5:
>   new patch
> ---
>  drivers/core/read.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 


Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

2023-03-06 Thread Simon Glass
Hi Johan,

On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
>
> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
> can expect 64-bit data from the device tree parser, so use

Why is that? It seems quite inefficient.

> dev_read_addr_index_ptr instead of the dev_read_addr_index function
> in the various files in the drivers directory that cast to a pointer.
>
> Signed-off-by: Johan Jonker 
> Reviewed-by: Michael Trimarchi 
> ---
>
> Changed V6:
>   use -EINVAL on return
>   drop cast
> ---
>  drivers/mtd/nand/raw/cortina_nand.c |  4 ++--
>  drivers/net/dm9000x.c   |  2 +-
>  drivers/net/dwmac_meson8b.c |  4 ++--
>  drivers/pci/pcie_dw_meson.c |  8 
>  drivers/pci/pcie_dw_rockchip.c  |  8 
>  drivers/watchdog/sbsa_gwdt.c| 12 ++--
>  6 files changed, 19 insertions(+), 19 deletions(-)

[..]

Regards,
SImon


Re: [PATCH v5 32/44] x86: Fix up use of X86_32BIT_INIT and X86_64 options

2023-03-06 Thread Simon Glass
Hi Tom,

On Fri, 3 Mar 2023 at 07:50, Tom Rini  wrote:
>
> On Wed, Feb 22, 2023 at 09:34:13AM -0700, Simon Glass wrote:
>
> > Drop the invalid SPL_ in a CONFIG_IS_ENABLED() usage. Use the correct
> > X86_64 option in msr.h since SPL may be 32-bit when U-Bout proper is
> > not.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > (no changes since v4)
> >
> > Changes in v4:
> > - Reduce and rename commit
> >
> >  arch/x86/cpu/qemu/qemu.c   | 2 +-
> >  arch/x86/include/asm/msr.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
>
> This changes spl on chromebook_link64 a lot, have you confirmed it
> there?

No, the lab is broken...will do at some point.

Regards,
Simon


Re: [PATCH V7 09/15] iot2050: Add script for signing artifacts

2023-03-06 Thread Simon Glass
Hi Jan,

On Tue, 28 Feb 2023 at 11:21, Jan Kiszka  wrote:
>
> From: Jan Kiszka 
>
> There are many ways to get a signed firmware for the IOT2050 devices,
> namely for the parts under user-control. This script documents one way
> of doing it, given a signing key. Augment the board documentation with
> the required procedure around it.
>
> Signed-off-by: Jan Kiszka 
> ---
>  doc/board/siemens/iot2050.rst | 52 +++
>  tools/iot2050-sign-fw.sh  | 51 ++
>  2 files changed, 103 insertions(+)
>  create mode 100755 tools/iot2050-sign-fw.sh

I sent a series which:

- attempts to do this with binman (providing x509 support)
- allows use of 'binman replace' in your script, by enhancing support
for updating sections

Please take a look at see what you think.

Regards,
SImon


Re: [PATCH V7 04/15] iot2050: Migrate settings into board env file

2023-03-06 Thread Simon Glass
Hi Jan,

On Wed, 1 Mar 2023 at 23:38, Jan Kiszka  wrote:
>
> On 02.03.23 00:38, Simon Glass wrote:
> > Hi Jan,
> >
> > On Tue, 28 Feb 2023 at 11:20, Jan Kiszka  wrote:
> >>
> >> From: Jan Kiszka 
> >>
> >> Anything that is not boot-env related is better kept there by now.
> >>
> >> At this chance, also drop a stale comment from iot2050.h
> >>
> >> Signed-off-by: Jan Kiszka 
> >> ---
> >>  board/siemens/iot2050/iot2050.env |  9 +
> >>  include/configs/iot2050.h | 11 ++-
> >>  2 files changed, 11 insertions(+), 9 deletions(-)
> >>  create mode 100644 board/siemens/iot2050/iot2050.env
> >>
> >> diff --git a/board/siemens/iot2050/iot2050.env 
> >> b/board/siemens/iot2050/iot2050.env
> >> new file mode 100644
> >> index 000..4bd93f0b2f4
> >> --- /dev/null
> >> +++ b/board/siemens/iot2050/iot2050.env
> >> @@ -0,0 +1,9 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Copyright (c) Siemens AG, 2023
> >> + *
> >> + * Authors:
> >> + *   Jan Kiszka 
> >> + */
> >> +
> >> +usb_pgood_delay=900
> >> diff --git a/include/configs/iot2050.h b/include/configs/iot2050.h
> >> index cfff46ce339..8dfeaddf541 100644
> >> --- a/include/configs/iot2050.h
> >> +++ b/include/configs/iot2050.h
> >> @@ -13,12 +13,6 @@
> >>
> >>  #include 
> >>
> >> -/* SPL Loader Configuration */
> >> -
> >> -/* U-Boot general configuration */
> >> -#define EXTRA_ENV_IOT2050_BOARD_SETTINGS   \
> >> -   "usb_pgood_delay=900\0"
> >> -
> >>  #if IS_ENABLED(CONFIG_CMD_USB)
> >>  # define BOOT_TARGET_USB(func) \
> >> func(USB, usb, 0) \
> >> @@ -40,10 +34,9 @@
> >>
> >>  #include 
> >>
> >> -#define CFG_EXTRA_ENV_SETTINGS \
> >> +#define CFG_EXTRA_ENV_SETTINGS \
> >> DEFAULT_LINUX_BOOT_ENV  \
> >> -   BOOTENV \
> >> -   EXTRA_ENV_IOT2050_BOARD_SETTINGS
> >> +   BOOTENV
> >>
> >>  #include 
> >>
> >> --
> >> 2.35.3
> >>
> >
> > You might want to move to standard boot so you can use a text-based
> > environment. See for example [1] [2] and later patches from [3].
> >
>
> Err, this patch is about introducing a text-based env for the parts that
> can be moved. I don't see a relevant delta after this patch to the
> referenced examples (btw, [2] is missing).

Sorry, yes. But if you move to standard boot then you don't need BOOTENV

[2] is https://patchwork.ozlabs.org/project/uboot/list/?series=344332=*

>
> Jan
>
> > Regards,
> > Simon
> >
> > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=342718
> > [2]
> > [3] from 
> > https://patchwork.ozlabs.org/project/uboot/list/?series=338993=*
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux
>

Regards,
SImon


Re: [PATCH v6 17/22] drivers: use devfdt_get_addr_size_index_ptr when cast to pointer

2023-03-06 Thread Simon Glass
On Thu, 2 Mar 2023 at 17:16, Johan Jonker  wrote:
>
> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
> can expect 64-bit data from the device tree parser, so use
> devfdt_get_addr_size_index_ptr instead of the devfdt_get_addr_size_index
> function in the various files in the drivers directory that cast to
> a pointer.
>
> Signed-off-by: Johan Jonker 
> Reviewed-by: Michael Trimarchi 
> ---
>  drivers/pci/pcie_dw_mvebu.c | 6 +++---
>  drivers/spi/cadence_qspi.c  | 3 +--
>  2 files changed, 4 insertions(+), 5 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH] cmd: fdt: Drop the 0x prefix

2023-03-06 Thread Simon Glass
Hi Marek,

On Wed, 1 Mar 2023 at 20:04, Marek Vasut
 wrote:
>
> The 'fdt get addr' is always assumed to be hex value, drop the prefix.
> Since this might break existing users who depend on the existing
> behavior with 0x prefix, this is a separate patch. Revert if this
> breaks anything.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Heinrich Schuchardt 
> Cc: Simon Glass 
> Cc: Tom Rini 
> ---
>  cmd/fdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cmd/fdt.c b/cmd/fdt.c
> index f38fe909c3e..04b664e652c 100644
> --- a/cmd/fdt.c
> +++ b/cmd/fdt.c
> @@ -478,7 +478,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int 
> argc, char *const argv[])
> /* Get address */
> char buf[19];
>
> -   snprintf(buf, sizeof(buf), "0x%lx",
> +   snprintf(buf, sizeof(buf), "%lx",
>  (ulong)map_to_sysmem(nodep));
> env_set(var, buf);
> } else if (subcmd[0] == 's') {
> --
> 2.39.2
>

iwc how about using env_sethex() ?

Regards,
Simon


Re: [PATCH 2/2] test: cmd: fdt: Drop new unneeded curly brackets

2023-03-06 Thread Simon Glass
On Wed, 1 Mar 2023 at 20:05, Marek Vasut
 wrote:
>
> Drop no longer needed { } around ut_assert*() functions in FDT test.
> No functional change.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Heinrich Schuchardt 
> Cc: Simon Glass 
> Cc: Tom Rini 
> ---
>  test/cmd/fdt.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 1/1] doc: man-page for panic command

2023-03-06 Thread Simon Glass
On Fri, 3 Mar 2023 at 14:51, Heinrich Schuchardt
 wrote:
>
> Provide a man-page for the panic command.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  doc/usage/cmd/panic.rst | 33 +
>  doc/usage/index.rst |  1 +
>  2 files changed, 34 insertions(+)
>  create mode 100644 doc/usage/cmd/panic.rst

Reviewed-by: Simon Glass 


Re: [PATCH] nvedit: simplify do_env_indirect()

2023-03-06 Thread Simon Glass
On Mon, 6 Mar 2023 at 06:27, Rasmus Villemoes
 wrote:
>
> Instead of calling env_get(from) up to three times, just do it once,
> computing the value we will put into 'to' and error out if that is
> NULL (i.e. no 'from' variable and no default provided).
>
> No functional change.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  cmd/nvedit.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH v6 12/22] core: fdtaddr: add devfdt_get_addr_size_index_ptr function

2023-03-06 Thread Simon Glass
Hi Johan,

On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
>
> Add devfdt_get_addr_size_index_ptr function with the same
> functionality as devfdt_get_addr_size_index, but instead
> a return pointer is given.
>
> Suggested-by: Michael Nazzareno Trimarchi 
> Signed-off-by: Johan Jonker 
> Reviewed-by: Michael Trimarchi 
> ---
>
> Changed V5:
>   fix spelling
>   use tabs
> ---
>  drivers/core/fdtaddr.c |  8 
>  include/dm/fdtaddr.h   | 17 -
>  2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
> index 91bcd1a2..f5906ff9 100644
> --- a/drivers/core/fdtaddr.c
> +++ b/drivers/core/fdtaddr.c
> @@ -122,6 +122,14 @@ fdt_addr_t devfdt_get_addr_size_index(const struct 
> udevice *dev, int index,
>  #endif
>  }
>
> +void *devfdt_get_addr_size_index_ptr(const struct udevice *dev, int index,
> +fdt_size_t *size)
> +{
> +   fdt_addr_t addr = devfdt_get_addr_size_index(dev, index, size);
> +
> +   return (addr == FDT_ADDR_T_NONE) ? NULL : (void *)(uintptr_t)addr;

Shouldn't this use map_to_sysmem()? We should not cast addresses to pointers.

[..]

Regards,
SImon


Re: [PATCH 1/2] test: Wrap assert macros in ({ ... }) and fix missing semicolons

2023-03-06 Thread Simon Glass
On Wed, 1 Mar 2023 at 20:05, Marek Vasut
 wrote:
>
> Wrap the assert macros in ({ ... }) so they can be safely used both as
> right side argument as well as in conditionals without curly brackets
> around them. In the process, find a bunch of missing semicolons, fix
> them.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Heinrich Schuchardt 
> Cc: Simon Glass 
> Cc: Tom Rini 
> ---
>  include/test/ut.h  | 152 ++---
>  test/cmd/pwm.c |   4 +-
>  test/dm/acpigen.c  |   2 +-
>  test/dm/misc.c |   4 +-
>  test/dm/phy.c  |   8 +--
>  test/dm/scmi.c |   4 +-
>  test/lib/kconfig.c |  10 +--
>  test/unicode_ut.c  |   6 +-
>  8 files changed, 121 insertions(+), 69 deletions(-)
>

Gosh. That was pretty bad. Thanks for fixing.

Reviewed-by: Simon Glass 


Re: [PATCH v2 02/32] cmd: fdt: Fix handling of empty properties for fdt get addr and fdt get size

2023-03-06 Thread Simon Glass
Hi Marek,

On Wed, 1 Mar 2023 at 20:09, Marek Vasut
 wrote:
>
> It is perfectly valid to request an address or size of FDT property
> without value, the only special case if requesting of the value of
> FDT property without value. Invert the test such, that properties
> without value still set the variable from 'fdt get addr/size' to
> address of the property or size of the property, where the later
> is 0.
>
> Signed-off-by: Marek Vasut 
> Reviewed-by: Simon Glass 
> ---
> Cc: Heinrich Schuchardt 
> Cc: Simon Glass 
> Cc: Tom Rini 
> ---
> V2: Add RB from Simon

BTW 'patman status' can collect review tags and add them to your
commits automatically.

Regards,
Simon


Re: [PATCH 23/32] test: cmd: fdt: Test fdt set

2023-03-06 Thread Simon Glass
On Wed, 1 Mar 2023 at 20:07, Marek Vasut  wrote:
>
> On 3/1/23 16:02, Simon Glass wrote:
> > Hi Marek,
> >
> > On Mon, 27 Feb 2023 at 12:55, Marek Vasut
> >  wrote:
> >>
> >> Add 'fdt set' test which works as follows:
> >> - Create fuller FDT, map it to sysmem
> >> - Set either existing property to overwrite it, or new property
> >> - Test setting both single properties as well as string and integer arrays
> >> - Test setting to non-existent nodes and aliases
> >> - Verify set values using 'fdt get value'
> >>
> >> The test case can be triggered using:
> >> "
> >> ./u-boot -Dc 'ut fdt'
> >> "
> >> To dump the full output from commands used during test, add '-v' flag.
> >>
> >> Signed-off-by: Marek Vasut 
> >> ---
> >> Cc: Heinrich Schuchardt 
> >> Cc: Simon Glass 
> >> Cc: Tom Rini 
> >> ---
> >>   test/cmd/fdt.c | 123 +
> >>   1 file changed, 123 insertions(+)
> >>
> >> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> >> index ae67b468b71..42d067090aa 100644
> >> --- a/test/cmd/fdt.c
> >> +++ b/test/cmd/fdt.c
> >> @@ -777,6 +777,129 @@ static int fdt_test_get_size(struct unit_test_state 
> >> *uts)
> >>   }
> >>   FDT_TEST(fdt_test_get_size, UT_TESTF_CONSOLE_REC);
> >>
> >> +static int fdt_test_set_single(struct unit_test_state *uts,
> >> +  const char *path, const char *prop,
> >> +  const char *sval, int ival, bool integer)
> >
> > Please  add a comment for this function.
> >
> >> +{
> >> +   ut_assertok(console_record_reset_enable());
> >> +   if (sval) {
> >> +   ut_assertok(run_commandf("fdt set %s %s %s", path, prop, 
> >> sval));
> >> +   } else if (integer) {
> >> +   ut_assertok(run_commandf("fdt set %s %s <%d>", path, prop, 
> >> ival));
> >> +   } else {
> >> +   ut_assertok(run_commandf("fdt set %s %s", path, prop));
> >> +   }
> >
> > Should drop {} on single-line statements - please check patman
>
> This one isn't as simple as "drop the {}" in fact, I sent a separate
> series to address that:
>
> https://patchwork.ozlabs.org/project/uboot/list/?series=344329

Oh yes, I hit that a while back.

Reviewed-by: Simon Glass 


Re: [PATCH v3 3/5] doc: document read/write commands

2023-03-06 Thread Simon Glass
On Thu, 2 Mar 2023 at 01:12, Rasmus Villemoes
 wrote:
>
> The read and write commands are, deliberately, implemented in the same
> file, so that they stay feature-compatible (e.g. if someone implements
> support for "read the full partition, however large that is", that
> same syntax should also work for write). In order to ensure the
> documentation for both are similarly kept in sync, and to avoid
> duplication, document them both in read.rst, and add a stub write.rst
> referring to read.rst.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  doc/usage/cmd/read.rst  | 44 +
>  doc/usage/cmd/write.rst |  6 ++
>  doc/usage/index.rst |  2 ++
>  3 files changed, 52 insertions(+)
>  create mode 100644 doc/usage/cmd/read.rst
>  create mode 100644 doc/usage/cmd/write.rst
>

Reviewed-by: Simon Glass 


[PATCH v4 12/14] arm64: dts: imx8mp: Drop EQoS clock workaround

2023-03-06 Thread Marek Vasut
The assigned-clock no longer have to be dropped, the clock are now
defined in clk-imx8mp.c and used by DWMAC driver to configure the
DWMAC clock. Drop the workarounds from U-Boot specific DT extras.

Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: No change
V3: No change
V4: No change
---
 arch/arm/dts/imx8mp-dhcom-u-boot.dtsi| 6 --
 arch/arm/dts/imx8mp-evk-u-boot.dtsi  | 6 --
 arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi | 6 --
 arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi| 6 --
 arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi  | 6 --
 5 files changed, 30 deletions(-)

diff --git a/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi 
b/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi
index ae838caebcf..ea6ab9f2e17 100644
--- a/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi
@@ -33,12 +33,6 @@
u-boot,dm-spl;
 };
 
- {
-   /delete-property/ assigned-clocks;
-   /delete-property/ assigned-clock-parents;
-   /delete-property/ assigned-clock-rates;
-};
-
  {
u-boot,dm-spl;
 };
diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi 
b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
index f43eb6238d0..cd0fb815c79 100644
--- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
@@ -131,12 +131,6 @@
u-boot,dm-spl;
 };
 
- {
-   /delete-property/ assigned-clocks;
-   /delete-property/ assigned-clock-parents;
-   /delete-property/ assigned-clock-rates;
-};
-
  {
reset-gpios = < 22 GPIO_ACTIVE_LOW>;
reset-delay-us = <15000>;
diff --git a/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi 
b/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi
index 342c523b0c5..3e48cf8ec5c 100644
--- a/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi
@@ -130,12 +130,6 @@
u-boot,dm-spl;
 };
 
- {
-   /delete-property/ assigned-clocks;
-   /delete-property/ assigned-clock-parents;
-   /delete-property/ assigned-clock-rates;
-};
-
  {
reset-gpios = < 22 GPIO_ACTIVE_LOW>;
reset-delay-us = <15000>;
diff --git a/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi 
b/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi
index d8721124526..849950fe026 100644
--- a/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi
@@ -20,12 +20,6 @@
};
 };
 
- {
-   /delete-property/ assigned-clocks;
-   /delete-property/ assigned-clock-parents;
-   /delete-property/ assigned-clock-rates;
-};
-
  {
reset-gpios = < 30 GPIO_ACTIVE_LOW>;
reset-delay-us = <1000>;
diff --git a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi 
b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
index 8a4cdc717d2..5f021d17230 100644
--- a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
@@ -39,12 +39,6 @@
u-boot,dm-spl;
 };
 
- {
-   /delete-property/ assigned-clocks;
-   /delete-property/ assigned-clock-parents;
-   /delete-property/ assigned-clock-rates;
-};
-
  {
u-boot,dm-spl;
 };
-- 
2.39.2



[PATCH v4 14/14] arm64: imx8mm: imx8mn: imx8mp: Drop FEC GPR[1] board workaround

2023-03-06 Thread Marek Vasut
The FEC interface mode is now configured in common board_interface_eth_init()
and called by FEC MAC driver when appropriate. Drop the board side duplicates
if the same functionality.

Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V3: New patch
V4: Drop ifdef MX8MP around imx8mp_fec_interface_init
---
 arch/arm/mach-imx/imx8m/clock_imx8mm.c| 47 ---
 .../dh_imx8mp/imx8mp_dhcom_pdk2.c | 12 -
 board/engicam/imx8mm/icore_mx8mm.c| 15 +-
 board/kontron/pitx_imx8m/pitx_imx8m.c | 14 +-
 4 files changed, 2 insertions(+), 86 deletions(-)

diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c 
b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
index e26658a08de..c380d9d2950 100644
--- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
+++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
@@ -875,53 +875,6 @@ static int imx8mp_eqos_interface_init(struct udevice *dev,
 #endif
 
 #ifdef CONFIG_FEC_MXC
-int set_clk_enet(enum enet_freq type)
-{
-   u32 target;
-   u32 enet1_ref;
-
-   switch (type) {
-   case ENET_125MHZ:
-   enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_125M_CLK;
-   break;
-   case ENET_50MHZ:
-   enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_50M_CLK;
-   break;
-   case ENET_25MHZ:
-   enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_25M_CLK;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   /* disable the clock first */
-   clock_enable(CCGR_ENET1, 0);
-   clock_enable(CCGR_SIM_ENET, 0);
-
-   /* set enet axi clock 266Mhz */
-   target = CLK_ROOT_ON | ENET_AXI_CLK_ROOT_FROM_SYS1_PLL_266M |
-CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
-CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1);
-   clock_set_target_val(ENET_AXI_CLK_ROOT, target);
-
-   target = CLK_ROOT_ON | enet1_ref |
-CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
-CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1);
-   clock_set_target_val(ENET_REF_CLK_ROOT, target);
-
-   target = CLK_ROOT_ON |
-   ENET1_TIME_CLK_ROOT_FROM_PLL_ENET_MAIN_100M_CLK |
-   CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
-   CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV4);
-   clock_set_target_val(ENET_TIMER_CLK_ROOT, target);
-
-   /* enable clock */
-   clock_enable(CCGR_SIM_ENET, 1);
-   clock_enable(CCGR_ENET1, 1);
-
-   return 0;
-}
-
 static int imx8mp_fec_interface_init(struct udevice *dev,
 phy_interface_t interface_type,
 bool mx8mp)
diff --git a/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c 
b/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c
index cb9973900bd..5edb85e1de5 100644
--- a/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c
+++ b/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c
@@ -37,17 +37,6 @@ int board_phys_sdram_size(phys_size_t *size)
return 0;
 }
 
-static void setup_fec(void)
-{
-   struct iomuxc_gpr_base_regs *gpr =
-   (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
-
-   /* Enable RGMII TX clk output. */
-   setbits_le32(>gpr[1], BIT(22));
-
-   set_clk_enet(ENET_125MHZ);
-}
-
 static int dh_imx8_setup_ethaddr(void)
 {
unsigned char enetaddr[6];
@@ -114,7 +103,6 @@ int dh_setup_mac_address(void)
 
 int board_init(void)
 {
-   setup_fec();
return 0;
 }
 
diff --git a/board/engicam/imx8mm/icore_mx8mm.c 
b/board/engicam/imx8mm/icore_mx8mm.c
index 4f7c699d7d1..320388faae3 100644
--- a/board/engicam/imx8mm/icore_mx8mm.c
+++ b/board/engicam/imx8mm/icore_mx8mm.c
@@ -29,7 +29,7 @@ static iomux_v3_cfg_t const fec1_rst_pads[] = {
IMX8MM_PAD_NAND_DATA01_GPIO3_IO7 | MUX_PAD_CTRL(NO_PAD_CTRL),
 };
 
-static void setup_iomux_fec(void)
+static void setup_fec(void)
 {
imx_iomux_v3_setup_multiple_pads(fec1_rst_pads,
 ARRAY_SIZE(fec1_rst_pads));
@@ -40,19 +40,6 @@ static void setup_iomux_fec(void)
gpio_direction_output(FEC_RST_PAD, 1);
 }
 
-static int setup_fec(void)
-{
-   struct iomuxc_gpr_base_regs *gpr =
-   (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
-
-   setup_iomux_fec();
-
-   /* Use 125M anatop REF_CLK1 for ENET1, not from external */
-   clrsetbits_le32(>gpr[1], 13, 0);
-
-   return set_clk_enet(ENET_125MHZ);
-}
-
 int board_phy_config(struct phy_device *phydev)
 {
/* enable rgmii rxc skew and phy mode select to RGMII copper */
diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c 
b/board/kontron/pitx_imx8m/pitx_imx8m.c

[PATCH v4 13/14] arm64: imx8mp: Drop EQoS GPR[1] board workaround

2023-03-06 Thread Marek Vasut
The EQoS interface mode is now configured in common board_interface_eth_init()
and called by EQoS MAC driver when appropriate. Drop the board side duplicates
if the same functionality.

Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: Fix the advantech board build
V3: Drop now unused architecture set_clk_eqos() code as well
V4: No change
---
 arch/arm/include/asm/arch-imx8m/clock.h   |  1 -
 arch/arm/mach-imx/imx8m/clock_imx8mm.c| 47 ---
 .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c   | 17 +--
 .../dh_imx8mp/imx8mp_dhcom_pdk2.c | 14 --
 board/engicam/imx8mp/icore_mx8mp.c| 16 ---
 board/freescale/imx8mp_evk/imx8mp_evk.c   | 17 ---
 board/gateworks/venice/venice.c   | 15 --
 board/msc/sm2s_imx8mp/sm2s_imx8mp.c   | 15 --
 board/toradex/verdin-imx8mp/verdin-imx8mp.c   | 16 ---
 9 files changed, 1 insertion(+), 157 deletions(-)

diff --git a/arch/arm/include/asm/arch-imx8m/clock.h 
b/arch/arm/include/asm/arch-imx8m/clock.h
index e4433763bc4..a861cd6db3a 100644
--- a/arch/arm/include/asm/arch-imx8m/clock.h
+++ b/arch/arm/include/asm/arch-imx8m/clock.h
@@ -276,5 +276,4 @@ int set_clk_qspi(void);
 void enable_ocotp_clk(unsigned char enable);
 int enable_i2c_clk(unsigned char enable, unsigned int i2c_num);
 int set_clk_enet(enum enet_freq type);
-int set_clk_eqos(enum enet_freq type);
 void hab_caam_clock_enable(unsigned char enable);
diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c 
b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
index 32f8623f235..e26658a08de 100644
--- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
+++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
@@ -827,53 +827,6 @@ u32 mxc_get_clock(enum mxc_clock clk)
 }
 
 #if defined(CONFIG_IMX8MP) && defined(CONFIG_DWC_ETH_QOS)
-int set_clk_eqos(enum enet_freq type)
-{
-   u32 target;
-   u32 enet1_ref;
-
-   switch (type) {
-   case ENET_125MHZ:
-   enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_125M_CLK;
-   break;
-   case ENET_50MHZ:
-   enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_50M_CLK;
-   break;
-   case ENET_25MHZ:
-   enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_25M_CLK;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   /* disable the clock first */
-   clock_enable(CCGR_QOS_ETHENET, 0);
-   clock_enable(CCGR_SDMA2, 0);
-
-   /* set enet axi clock 266Mhz */
-   target = CLK_ROOT_ON | ENET_AXI_CLK_ROOT_FROM_SYS1_PLL_266M |
-CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
-CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1);
-   clock_set_target_val(ENET_AXI_CLK_ROOT, target);
-
-   target = CLK_ROOT_ON | enet1_ref |
-CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
-CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1);
-   clock_set_target_val(ENET_QOS_CLK_ROOT, target);
-
-   target = CLK_ROOT_ON |
-   ENET1_TIME_CLK_ROOT_FROM_PLL_ENET_MAIN_100M_CLK |
-   CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
-   CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV4);
-   clock_set_target_val(ENET_QOS_TIMER_CLK_ROOT, target);
-
-   /* enable clock */
-   clock_enable(CCGR_QOS_ETHENET, 1);
-   clock_enable(CCGR_SDMA2, 1);
-
-   return 0;
-}
-
 static int imx8mp_eqos_interface_init(struct udevice *dev,
  phy_interface_t interface_type)
 {
diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c 
b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
index 34109c69ddb..9191ddbb682 100644
--- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
+++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c
@@ -113,7 +113,7 @@ static const iomux_v3_cfg_t eqos_rst_pads[] = {
MX8MP_PAD_SAI2_RXC__GPIO4_IO22 | MUX_PAD_CTRL(NO_PAD_CTRL),
 };
 
-static void setup_iomux_eqos(void)
+static void setup_eqos(void)
 {
imx_iomux_v3_setup_multiple_pads(eqos_rst_pads,
 ARRAY_SIZE(eqos_rst_pads));
@@ -124,21 +124,6 @@ static void setup_iomux_eqos(void)
gpio_direction_output(EQOS_RST_PAD, 1);
mdelay(100);
 }
-
-static int setup_eqos(void)
-{
-   struct iomuxc_gpr_base_regs *gpr =
-   (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
-
-   setup_iomux_eqos();
-
-   /* set INTF as RGMII, enable RGMII TXC clock */
-   clrsetbits_le32(>gpr[1],
-   IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK, BIT(16));
-   setbits_le32(>gpr[1], BIT(19) | BIT(21));
-
-   return set_clk_eqos(ENET_125MHZ);
-}
 #endif /* 

[PATCH v4 08/14] net: dwc_eth_qos: Add i.MX8M Plus RMII support

2023-03-06 Thread Marek Vasut
With DM clock support in place, it is easy to add RMII support into the
MAC driver. The RMII cannot operate at 1000 Mbps and at 100 and 10 Mbps
the clock frequency is 50 MHz and 5 MHz instead of 25 MHz and 2.5 MHz.

The board DT requires the following adjustments to EQoS node:
  phy-mode = "rmii";
  assigned-clock-parents = < IMX8MP_SYS_PLL1_266M>,
< IMX8MP_SYS_PLL2_100M>,
< IMX8MP_SYS_PLL2_50M>;
  assigned-clock-rates = <0>, <1>, <5000>;

Reviewed-by: Ramon Fried 
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: Add RB from Ramon
V3: Handle RGMII_*ID variants
V4: No change
---
 drivers/net/dwc_eth_qos_imx.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dwc_eth_qos_imx.c b/drivers/net/dwc_eth_qos_imx.c
index f5f3f2099f0..962c5373243 100644
--- a/drivers/net/dwc_eth_qos_imx.c
+++ b/drivers/net/dwc_eth_qos_imx.c
@@ -179,21 +179,28 @@ static int eqos_set_tx_clk_speed_imx(struct udevice *dev)
 
debug("%s(dev=%p):\n", __func__, dev);
 
-   switch (eqos->phy->speed) {
-   case SPEED_1000:
-   rate = 125 * 1000 * 1000;
-   break;
-   case SPEED_100:
-   rate = 25 * 1000 * 1000;
-   break;
-   case SPEED_10:
-   rate = 2.5 * 1000 * 1000;
-   break;
-   default:
+   if (eqos->phy->interface == PHY_INTERFACE_MODE_RMII)
+   rate = 5000;/* 5000 kHz = 5 MHz */
+   else
+   rate = 2500;/* 2500 kHz = 2.5 MHz */
+
+   if (eqos->phy->speed == SPEED_1000 &&
+   (eqos->phy->interface == PHY_INTERFACE_MODE_RGMII ||
+eqos->phy->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+eqos->phy->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
+eqos->phy->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
+   rate *= 50; /* Use 50x base rate i.e. 125 MHz */
+   } else if (eqos->phy->speed == SPEED_100) {
+   rate *= 10; /* Use 10x base rate */
+   } else if (eqos->phy->speed == SPEED_10) {
+   rate *= 1;  /* Use base rate */
+   } else {
pr_err("invalid speed %d", eqos->phy->speed);
return -EINVAL;
}
 
+   rate *= 1000;   /* clk_set_rate() operates in Hz */
+
ret = clk_set_rate(>clk_tx, rate);
if (ret < 0) {
pr_err("imx (tx_clk, %lu) failed: %d", rate, ret);
-- 
2.39.2



[PATCH v4 09/14] net: dwc_eth_qos: Add board_interface_eth_init() for i.MX8M Plus

2023-03-06 Thread Marek Vasut
Implement common board_interface_eth_init() and call it from the DWMAC
driver to configure IOMUXC GPR[1] register according to the PHY mode
obtained from DT. This supports all three interface modes supported by
the i.MX8M Plus DWMAC and supersedes current board-side configuration
of the same IOMUX GPR[1] duplicated in the board files.

Reviewed-by: Ramon Fried 
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: - Add RB from Ramon
- Handle RGMII_ID/RGMII_RXID/RGMII_TXID just like plain RGMII
V3: Make the function more generic, so it can be shared by eqos and fec
V4: No change
---
 arch/arm/include/asm/arch-imx8m/imx-regs.h |  8 ++-
 arch/arm/mach-imx/imx8m/clock_imx8mm.c | 59 +-
 drivers/net/dwc_eth_qos_imx.c  |  4 ++
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h 
b/arch/arm/include/asm/arch-imx8m/imx-regs.h
index 1559bf6d218..1818b459fa6 100644
--- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
+++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
@@ -89,7 +89,13 @@
 #define DDRC_IPS_BASE_ADDR(X)  (0x3d40 + ((X) * 0x200))
 #define DDR_CSD1_BASE_ADDR 0x4000
 
-#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK 0x7
+#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN  BIT(21)
+#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SELBIT(20)
+#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_ENBIT(19)
+#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK GENMASK(18, 16)
+#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MII  (0 << 16)
+#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RGMII(1 << 16)
+#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RMII (4 << 16)
 #define FEC_QUIRK_ENET_MAC
 
 #ifdef CONFIG_ARMV8_PSCI   /* Final jump location */
diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c 
b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
index 494bfbedc8c..1546c9ba9a0 100644
--- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
+++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -825,7 +826,7 @@ u32 mxc_get_clock(enum mxc_clock clk)
return 0;
 }
 
-#ifdef CONFIG_DWC_ETH_QOS
+#if defined(CONFIG_IMX8MP) && defined(CONFIG_DWC_ETH_QOS)
 int set_clk_eqos(enum enet_freq type)
 {
u32 target;
@@ -872,6 +873,52 @@ int set_clk_eqos(enum enet_freq type)
 
return 0;
 }
+
+static int imx8mp_eqos_interface_init(struct udevice *dev,
+ phy_interface_t interface_type)
+{
+   struct iomuxc_gpr_base_regs *gpr =
+   (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
+
+   clrbits_le32(>gpr[1],
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SEL |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN);
+
+   switch (interface_type) {
+   case PHY_INTERFACE_MODE_MII:
+   setbits_le32(>gpr[1],
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MII);
+   break;
+   case PHY_INTERFACE_MODE_RMII:
+   setbits_le32(>gpr[1],
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SEL |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RMII);
+   break;
+   case PHY_INTERFACE_MODE_RGMII:
+   case PHY_INTERFACE_MODE_RGMII_ID:
+   case PHY_INTERFACE_MODE_RGMII_RXID:
+   case PHY_INTERFACE_MODE_RGMII_TXID:
+   setbits_le32(>gpr[1],
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN |
+IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RGMII);
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+#else
+static int imx8mp_eqos_interface_init(struct udevice *dev,
+ phy_interface_t interface_type)
+{
+   return 0;
+}
 #endif
 
 #ifdef CONFIG_FEC_MXC
@@ -922,3 +969,13 @@ int set_clk_enet(enum enet_freq type)
return 0;
 }
 #endif
+
+int board_interface_eth_init(struct udevice *dev, phy_interface_t 
interface_type)
+{
+   if (IS_ENABLED(CONFIG_IMX8MP) &&
+   IS_ENABLED(CONFIG_DWC_ETH_QOS) &&
+   device_is_compatible(dev, "nxp,imx8mp-dwmac-eqos"))
+   return imx8mp_eqos_interface_init(dev, 

[PATCH v4 06/14] net: dwc_eth_qos: Set DMA_MODE SWR bit to reset the MAC

2023-03-06 Thread Marek Vasut
The driver currently only waits for DMA_MODE SWR bit to clear itself.
This is insufficient e.g. on i.MX8M Plus, where the MAC must be reset
before IOMUX GPR[1] content is latched into the MAC and used. Without
the proper reset, the i.MX8M Plus MAC variant does not take the value
in IOMUX GPR[1] into account, which makes it impossible e.g. to switch
interface mode from RGMII to any other.

Since proper reset is desired in general to put the block into defined
state, always assert the DMA_MODE SWR bit before waiting for the bit
to clear itself.

Reviewed-by: Ramon Fried 
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: Add RB from Ramon
V3: No change
V4: No change
---
 drivers/net/dwc_eth_qos.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
index 9a5575e7b83..ec58697b311 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -761,6 +761,12 @@ static int eqos_start(struct udevice *dev)
 
eqos->reg_access_ok = true;
 
+   /*
+* Assert the SWR first, the actually reset the MAC and to latch in
+* e.g. i.MX8M Plus GPR[1] content, which selects interface mode.
+*/
+   setbits_le32(>dma_regs->mode, EQOS_DMA_MODE_SWR);
+
ret = wait_for_bit_le32(>dma_regs->mode,
EQOS_DMA_MODE_SWR, false,
eqos->config->swr_wait, false);
-- 
2.39.2



[PATCH v4 11/14] net: fec_mxc: Add board_interface_eth_init() for i.MX8M Mini/Nano/Plus

2023-03-06 Thread Marek Vasut
Implement common board_interface_eth_init() and call it from the FEC
driver to configure IOMUXC GPR[1] register according to the PHY mode
obtained from DT. This supports all three interface modes supported by
the i.MX8M Mini/Nano/Plus FEC and supersedes the current board-side
configuration of the same IOMUX GPR[1] duplicated in the board files.

Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V3: New patch
V4: Drop ifdef MX8MP around imx8mp_fec_interface_init
---
 arch/arm/include/asm/arch-imx8m/imx-regs.h |  2 +
 arch/arm/mach-imx/imx8m/clock_imx8mm.c | 46 ++
 drivers/net/fec_mxc.c  |  4 ++
 3 files changed, 52 insertions(+)

diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h 
b/arch/arm/include/asm/arch-imx8m/imx-regs.h
index 1818b459fa6..6e2fc82a0e4 100644
--- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
+++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
@@ -89,6 +89,7 @@
 #define DDRC_IPS_BASE_ADDR(X)  (0x3d40 + ((X) * 0x200))
 #define DDR_CSD1_BASE_ADDR 0x4000
 
+#define IOMUXC_GPR_GPR1_GPR_ENET1_RGMII_EN BIT(22)
 #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN  BIT(21)
 #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SELBIT(20)
 #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_ENBIT(19)
@@ -96,6 +97,7 @@
 #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MII  (0 << 16)
 #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RGMII(1 << 16)
 #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RMII (4 << 16)
+#define IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL   BIT(13)
 #define FEC_QUIRK_ENET_MAC
 
 #ifdef CONFIG_ARMV8_PSCI   /* Final jump location */
diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c 
b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
index 1546c9ba9a0..32f8623f235 100644
--- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
+++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
@@ -968,10 +968,56 @@ int set_clk_enet(enum enet_freq type)
 
return 0;
 }
+
+static int imx8mp_fec_interface_init(struct udevice *dev,
+phy_interface_t interface_type,
+bool mx8mp)
+{
+   /* i.MX8MP has extra RGMII_EN bit in IOMUXC GPR1 register */
+   const u32 rgmii_en = mx8mp ? IOMUXC_GPR_GPR1_GPR_ENET1_RGMII_EN : 0;
+   struct iomuxc_gpr_base_regs *gpr =
+   (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
+
+   clrbits_le32(>gpr[1],
+rgmii_en |
+IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL);
+
+   switch (interface_type) {
+   case PHY_INTERFACE_MODE_MII:
+   case PHY_INTERFACE_MODE_RMII:
+   setbits_le32(>gpr[1], 
IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL);
+   break;
+   case PHY_INTERFACE_MODE_RGMII:
+   case PHY_INTERFACE_MODE_RGMII_ID:
+   case PHY_INTERFACE_MODE_RGMII_RXID:
+   case PHY_INTERFACE_MODE_RGMII_TXID:
+   setbits_le32(>gpr[1], rgmii_en);
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
 #endif
 
 int board_interface_eth_init(struct udevice *dev, phy_interface_t 
interface_type)
 {
+   if (IS_ENABLED(CONFIG_IMX8MM) &&
+   IS_ENABLED(CONFIG_FEC_MXC) &&
+   device_is_compatible(dev, "fsl,imx8mm-fec"))
+   return imx8mp_fec_interface_init(dev, interface_type, false);
+
+   if (IS_ENABLED(CONFIG_IMX8MN) &&
+   IS_ENABLED(CONFIG_FEC_MXC) &&
+   device_is_compatible(dev, "fsl,imx8mn-fec"))
+   return imx8mp_fec_interface_init(dev, interface_type, false);
+
+   if (IS_ENABLED(CONFIG_IMX8MP) &&
+   IS_ENABLED(CONFIG_FEC_MXC) &&
+   device_is_compatible(dev, "fsl,imx8mp-fec"))
+   return imx8mp_fec_interface_init(dev, interface_type, true);
+
if (IS_ENABLED(CONFIG_IMX8MP) &&
IS_ENABLED(CONFIG_DWC_ETH_QOS) &&
device_is_compatible(dev, "nxp,imx8mp-dwmac-eqos"))
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 7a8577158ae..ac937676f9c 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1232,6 +1232,10 @@ static int fecmxc_probe(struct udevice *dev)
uint32_t start;
int ret;
 
+   ret = board_interface_eth_init(dev, pdata->phy_interface);
+   if (ret)
+   return ret;
+
if (IS_ENABLED(CONFIG_IMX_MODULE_FUSE)) {
if (enet_fused((ulong)priv->eth)) {
printf("SoC fuse indicates Ethernet@0x%lx is 
unavailable.\n", (ulong)priv->eth);
-- 
2.39.2



[PATCH v4 10/14] net: fec_mxc: Add ref clock setup support for i.MX8M Mini/Nano/Plus

2023-03-06 Thread Marek Vasut
The FEC ref clock frequency on i.MX8M Mini/Nano/Plus was so far configured
via ad-hoc board code. Replace that with DM clock clk_set_rate() instead.
This way, the driver claims all its required clock and sets the ref clock
rate, without any need of architecture specific register fiddling.

Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V3: New patch
V4: No change
---
 drivers/net/fec_mxc.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 1a6c18a441f..7a8577158ae 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1196,6 +1196,33 @@ static void fec_gpio_reset(struct fec_priv *priv)
 }
 #endif
 
+static int fecmxc_set_ref_clk(struct clk *clk_ref, phy_interface_t interface)
+{
+   unsigned int freq;
+   int ret;
+
+   if (!CONFIG_IS_ENABLED(CLK_CCF))
+   return 0;
+
+   if (interface == PHY_INTERFACE_MODE_MII)
+   freq = 2500;
+   else if (interface == PHY_INTERFACE_MODE_RMII)
+   freq = 5000;
+   else if (interface == PHY_INTERFACE_MODE_RGMII ||
+interface == PHY_INTERFACE_MODE_RGMII_ID ||
+interface == PHY_INTERFACE_MODE_RGMII_RXID ||
+interface == PHY_INTERFACE_MODE_RGMII_TXID)
+   freq = 12500;
+   else
+   return -EINVAL;
+
+   ret = clk_set_rate(clk_ref, freq);
+   if (ret < 0)
+   return ret;
+
+   return 0;
+}
+
 static int fecmxc_probe(struct udevice *dev)
 {
bool dm_mii_bus = true;
@@ -1253,6 +1280,11 @@ static int fecmxc_probe(struct udevice *dev)
 
ret = clk_get_by_name(dev, "enet_clk_ref", >clk_ref);
if (!ret) {
+   ret = fecmxc_set_ref_clk(>clk_ref,
+pdata->phy_interface);
+   if (ret)
+   return ret;
+
ret = clk_enable(>clk_ref);
if (ret)
return ret;
-- 
2.39.2



[PATCH v4 02/14] net: Pull board_interface_eth_init() into common code

2023-03-06 Thread Marek Vasut
Move the board_interface_eth_init() into common ethernet uclass code,
since this function could be shared by multiple drivers.

Reviewed-by: Simon Glass 
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V3: New patch
V4: Add RB from Simon
---
 drivers/net/dwc_eth_qos.c | 7 ---
 net/eth-uclass.c  | 7 +++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
index 112deb546de..0cae2cf2064 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -1412,13 +1412,6 @@ err_free_reset_eqos:
return ret;
 }
 
-/* board-specific Ethernet Interface initializations. */
-__weak int board_interface_eth_init(struct udevice *dev,
-   phy_interface_t interface_type)
-{
-   return 0;
-}
-
 static int eqos_probe_resources_stm32(struct udevice *dev)
 {
struct eqos_priv *eqos = dev_get_priv(dev);
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index b01a910938e..c393600fabc 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -49,6 +49,13 @@ struct eth_uclass_priv {
 /* eth_errno - This stores the most recent failure code from DM functions */
 static int eth_errno;
 
+/* board-specific Ethernet Interface initializations. */
+__weak int board_interface_eth_init(struct udevice *dev,
+   phy_interface_t interface_type)
+{
+   return 0;
+}
+
 static struct eth_uclass_priv *eth_get_uclass_priv(void)
 {
struct uclass *uc;
-- 
2.39.2



[PATCH v4 04/14] net: dwc_eth_qos: Drop unused dm_gpio_free() on STM32

2023-03-06 Thread Marek Vasut
The dm_gpio_free() is never called, because for stm32, the phy_reset_gpio
pointer is never valid. This is because only tegra186 ever claims the
phy_reset_gpio, all other platforms use the PHY framework to reset the
PHY instead. Drop the dm_gpio_free() and dm_gpio_is_valid().

Reviewed-by: Ramon Fried 
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: - Add RB from Ramon
- Mark eqos variable in eqos_remove_resources_stm32() with __maybe_unused
V3: No change
V4: No change
---
 drivers/net/dwc_eth_qos.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
index 00690b28ca6..b97b3ea2db6 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -1493,7 +1493,7 @@ static int eqos_remove_resources_tegra186(struct udevice 
*dev)
 
 static int eqos_remove_resources_stm32(struct udevice *dev)
 {
-   struct eqos_priv *eqos = dev_get_priv(dev);
+   struct eqos_priv * __maybe_unused eqos = dev_get_priv(dev);
 
debug("%s(dev=%p):\n", __func__, dev);
 
@@ -1505,9 +1505,6 @@ static int eqos_remove_resources_stm32(struct udevice 
*dev)
clk_free(>clk_ck);
 #endif
 
-   if (dm_gpio_is_valid(>phy_reset_gpio))
-   dm_gpio_free(dev, >phy_reset_gpio);
-
debug("%s: OK\n", __func__);
return 0;
 }
-- 
2.39.2



[PATCH v4 07/14] net: dwc_eth_qos: Add DM CLK support for i.MX8M Plus

2023-03-06 Thread Marek Vasut
The DWMAC clock in i.MX8M Plus were so far configured via ad-hoc
architecture code. Replace that with DM clock instead. This way,
the driver claims all its required clock, enables and disables
them, and even gets the CSR clock rate and sets the TX clock rate,
without any need of architecture specific register fiddling. Drop
the architecture specific code while at it too.

The adjustment here is modeled after STM32MP15xx clock handling
in this driver.

Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: Turn all the pr_err() into dev_dbg()
V3: No change
V4: No change
---
 arch/arm/mach-imx/imx8m/clock_imx8mm.c |  41 
 drivers/net/dwc_eth_qos_imx.c  | 131 +++--
 2 files changed, 121 insertions(+), 51 deletions(-)

diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c 
b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
index 64ad57e9b39..494bfbedc8c 100644
--- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
+++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
@@ -872,47 +872,6 @@ int set_clk_eqos(enum enet_freq type)
 
return 0;
 }
-
-int imx_eqos_txclk_set_rate(ulong rate)
-{
-   u32 val;
-   u32 eqos_post_div;
-
-   /* disable the clock first */
-   clock_enable(CCGR_QOS_ETHENET, 0);
-   clock_enable(CCGR_SDMA2, 0);
-
-   switch (rate) {
-   case 12500:
-   eqos_post_div = 1;
-   break;
-   case 2500:
-   eqos_post_div = 12500 / 2500;
-   break;
-   case 250:
-   eqos_post_div = 12500 / 250;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   clock_get_target_val(ENET_QOS_CLK_ROOT, );
-   val &= ~(CLK_ROOT_PRE_DIV_MASK | CLK_ROOT_POST_DIV_MASK);
-   val |= CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) |
-  CLK_ROOT_POST_DIV(eqos_post_div - 1);
-   clock_set_target_val(ENET_QOS_CLK_ROOT, val);
-
-   /* enable clock */
-   clock_enable(CCGR_QOS_ETHENET, 1);
-   clock_enable(CCGR_SDMA2, 1);
-
-   return 0;
-}
-
-u32 imx_get_eqos_csr_clk(void)
-{
-   return get_root_clk(ENET_AXI_CLK_ROOT);
-}
 #endif
 
 #ifdef CONFIG_FEC_MXC
diff --git a/drivers/net/dwc_eth_qos_imx.c b/drivers/net/dwc_eth_qos_imx.c
index 42cb164ad14..f5f3f2099f0 100644
--- a/drivers/net/dwc_eth_qos_imx.c
+++ b/drivers/net/dwc_eth_qos_imx.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -32,20 +33,18 @@ __weak u32 imx_get_eqos_csr_clk(void)
return 100 * 100;
 }
 
-__weak int imx_eqos_txclk_set_rate(unsigned long rate)
-{
-   return 0;
-}
-
 static ulong eqos_get_tick_clk_rate_imx(struct udevice *dev)
 {
-   return imx_get_eqos_csr_clk();
+   struct eqos_priv *eqos = dev_get_priv(dev);
+
+   return clk_get_rate(>clk_master_bus);
 }
 
 static int eqos_probe_resources_imx(struct udevice *dev)
 {
struct eqos_priv *eqos = dev_get_priv(dev);
phy_interface_t interface;
+   int ret;
 
debug("%s(dev=%p):\n", __func__, dev);
 
@@ -56,6 +55,118 @@ static int eqos_probe_resources_imx(struct udevice *dev)
return -EINVAL;
}
 
+   eqos->max_speed = dev_read_u32_default(dev, "max-speed", 0);
+
+   ret = clk_get_by_name(dev, "stmmaceth", >clk_master_bus);
+   if (ret) {
+   dev_dbg(dev, "clk_get_by_name(master_bus) failed: %d", ret);
+   goto err_probe;
+   }
+
+   ret = clk_get_by_name(dev, "ptp_ref", >clk_ptp_ref);
+   if (ret) {
+   dev_dbg(dev, "clk_get_by_name(ptp_ref) failed: %d", ret);
+   goto err_free_clk_master_bus;
+   }
+
+   ret = clk_get_by_name(dev, "tx", >clk_tx);
+   if (ret) {
+   dev_dbg(dev, "clk_get_by_name(tx) failed: %d", ret);
+   goto err_free_clk_ptp_ref;
+   }
+
+   ret = clk_get_by_name(dev, "pclk", >clk_ck);
+   if (ret) {
+   dev_dbg(dev, "clk_get_by_name(pclk) failed: %d", ret);
+   goto err_free_clk_tx;
+   }
+
+   debug("%s: OK\n", __func__);
+   return 0;
+
+err_free_clk_tx:
+   clk_free(>clk_tx);
+err_free_clk_ptp_ref:
+   clk_free(>clk_ptp_ref);
+err_free_clk_master_bus:
+   clk_free(>clk_master_bus);
+err_probe:
+
+   debug("%s: returns %d\n", __func__, ret);
+   return ret;
+}
+
+static int eqos_remove_resources_imx(struct udevice *dev)
+{
+   struct eqos_priv *eqos = dev_get_priv(dev);
+
+   debug("%s(dev=%p):\n", __func__, dev);
+
+   clk_free(>clk_ck);
+   clk_free(>clk_tx);
+   clk_free(>clk_ptp_ref);
+   clk_free(>clk_master_bus);
+
+   debug("%s: OK\n", __func__);
+ 

[PATCH v4 05/14] net: dwc_eth_qos: Staticize eqos_inval_buffer_tegra186()

2023-03-06 Thread Marek Vasut
This function is only used within the driver, staticize it.

Fixes: 149e80f74b6 ("net: dwc_eth_qos: public some functions")
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: - New patch
V3: No change
V4: No change
---
 drivers/net/dwc_eth_qos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
index b97b3ea2db6..9a5575e7b83 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -108,7 +108,7 @@ void eqos_flush_desc_generic(void *desc)
flush_dcache_range(start, end);
 }
 
-void eqos_inval_buffer_tegra186(void *buf, size_t size)
+static void eqos_inval_buffer_tegra186(void *buf, size_t size)
 {
unsigned long start = (unsigned long)buf & ~(ARCH_DMA_MINALIGN - 1);
unsigned long end = ALIGN(start + size, ARCH_DMA_MINALIGN);
-- 
2.39.2



[PATCH v4 01/14] clk: imx8mp: Add EQoS MAC clock

2023-03-06 Thread Marek Vasut
Add clock for the DWMAC EQoS block. This is used among other things
to configure the MII clock via DM CLK.

Acked-by: Sean Anderson 
Reviewed-by: Peng Fan 
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: Add AB from Sean
V3: No change
V4: No change
---
 drivers/clk/imx/clk-imx8mp.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index ffbc1d1ba9f..6dda0403e35 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -70,6 +70,14 @@ static const char *imx8mp_i2c6_sels[] = {"clock-osc-24m", 
"sys_pll1_160m", "sys_
 "sys_pll3_out", "audio_pll1_out", 
"video_pll1_out",
 "audio_pll2_out", "sys_pll1_133m", };
 
+static const char *imx8mp_enet_qos_sels[] = {"clock-osc-24m", "sys_pll2_125m", 
"sys_pll2_50m",
+"sys_pll2_100m", "sys_pll1_160m", 
"audio_pll1_out",
+"video_pll1_out", "clk_ext4", };
+
+static const char *imx8mp_enet_qos_timer_sels[] = {"clock-osc-24m", 
"sys_pll2_100m", "audio_pll1_out",
+  "clk_ext1", "clk_ext2", 
"clk_ext3",
+  "clk_ext4", 
"video_pll1_out", };
+
 static const char *imx8mp_usdhc1_sels[] = {"clock-osc-24m", "sys_pll1_400m", 
"sys_pll1_800m",
   "sys_pll2_500m", "sys_pll3_out", 
"sys_pll1_266m",
   "audio_pll2_out", "sys_pll1_100m", };
@@ -250,6 +258,8 @@ static int imx8mp_clk_probe(struct udevice *dev)
clk_dm(IMX8MP_CLK_DRAM_APB, imx8m_clk_composite_critical("dram_apb", 
imx8mp_dram_apb_sels, base + 0xa080));
clk_dm(IMX8MP_CLK_I2C5, imx8m_clk_composite("i2c5", imx8mp_i2c5_sels, 
base + 0xa480));
clk_dm(IMX8MP_CLK_I2C6, imx8m_clk_composite("i2c6", imx8mp_i2c6_sels, 
base + 0xa500));
+   clk_dm(IMX8MP_CLK_ENET_QOS, imx8m_clk_composite("enet_qos", 
imx8mp_enet_qos_sels, base + 0xa880));
+   clk_dm(IMX8MP_CLK_ENET_QOS_TIMER, imx8m_clk_composite("enet_qos_timer", 
imx8mp_enet_qos_timer_sels, base + 0xa900));
clk_dm(IMX8MP_CLK_ENET_REF, imx8m_clk_composite("enet_ref", 
imx8mp_enet_ref_sels, base + 0xa980));
clk_dm(IMX8MP_CLK_ENET_TIMER, imx8m_clk_composite("enet_timer", 
imx8mp_enet_timer_sels, base + 0xaa00));
clk_dm(IMX8MP_CLK_ENET_PHY_REF, imx8m_clk_composite("enet_phy_ref", 
imx8mp_enet_phy_ref_sels, base + 0xaa80));
@@ -292,10 +302,13 @@ static int imx8mp_clk_probe(struct udevice *dev)
clk_dm(IMX8MP_CLK_I2C2_ROOT, imx_clk_gate4("i2c2_root_clk", "i2c2", 
base + 0x4180, 0));
clk_dm(IMX8MP_CLK_I2C3_ROOT, imx_clk_gate4("i2c3_root_clk", "i2c3", 
base + 0x4190, 0));
clk_dm(IMX8MP_CLK_I2C4_ROOT, imx_clk_gate4("i2c4_root_clk", "i2c4", 
base + 0x41a0, 0));
+   clk_dm(IMX8MP_CLK_QOS_ROOT, imx_clk_gate4("qos_root_clk", "ipg_root", 
base + 0x42c0, 0));
+   clk_dm(IMX8MP_CLK_QOS_ENET_ROOT, imx_clk_gate4("qos_enet_root_clk", 
"ipg_root", base + 0x42e0, 0));
clk_dm(IMX8MP_CLK_QSPI_ROOT, imx_clk_gate4("qspi_root_clk", "qspi", 
base + 0x42f0, 0));
clk_dm(IMX8MP_CLK_I2C5_ROOT, imx_clk_gate2("i2c5_root_clk", "i2c5", 
base + 0x4330, 0));
clk_dm(IMX8MP_CLK_I2C6_ROOT, imx_clk_gate2("i2c6_root_clk", "i2c6", 
base + 0x4340, 0));
clk_dm(IMX8MP_CLK_SIM_ENET_ROOT, imx_clk_gate4("sim_enet_root_clk", 
"enet_axi", base + 0x4400, 0));
+   clk_dm(IMX8MP_CLK_ENET_QOS_ROOT, imx_clk_gate4("enet_qos_root_clk", 
"sim_enet_root_clk", base + 0x43b0, 0));
clk_dm(IMX8MP_CLK_UART1_ROOT, imx_clk_gate4("uart1_root_clk", "uart1", 
base + 0x4490, 0));
clk_dm(IMX8MP_CLK_UART2_ROOT, imx_clk_gate4("uart2_root_clk", "uart2", 
base + 0x44a0, 0));
clk_dm(IMX8MP_CLK_UART3_ROOT, imx_clk_gate4("uart3_root_clk", "uart3", 
base + 0x44b0, 0));
-- 
2.39.2



[PATCH v4 03/14] net: dwc_eth_qos: Drop bogus return after goto

2023-03-06 Thread Marek Vasut
The return is never triggered due to the goto just above it.
Drop it. No functional change.

Reviewed-by: Ramon Fried 
Signed-off-by: Marek Vasut 
---
Cc: "Ariel D'Alessandro" 
Cc: "NXP i.MX U-Boot Team" 
Cc: Andrey Zhizhikin 
Cc: Fabio Estevam 
Cc: Joe Hershberger 
Cc: Lukasz Majewski 
Cc: Marcel Ziswiler 
Cc: Marek Vasut 
Cc: Michael Trimarchi 
Cc: Peng Fan 
Cc: Ramon Fried 
Cc: Sean Anderson 
Cc: Stefano Babic 
Cc: Tim Harvey 
Cc: Tommaso Merciai 
Cc: u-boot@lists.denx.de
---
V2: Add RB from Ramon
V3: No change
V4: No change
---
 drivers/net/dwc_eth_qos.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
index 0cae2cf2064..00690b28ca6 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -1383,7 +1383,6 @@ static int eqos_probe_resources_tegra186(struct udevice 
*dev)
if (ret) {
pr_err("clk_get_by_name(ptp_ref) failed: %d", ret);
goto err_free_clk_rx;
-   return ret;
}
 
ret = clk_get_by_name(dev, "tx", >clk_tx);
-- 
2.39.2



Re: [PATCH 1/1] api: move API related config options into submenu

2023-03-06 Thread Tom Rini
On Mon, Mar 06, 2023 at 11:18:17AM +0100, Heinrich Schuchardt wrote:
> On 3/4/23 16:32, Tom Rini wrote:
> > On Fri, Mar 03, 2023 at 11:31:22PM +0100, Heinrich Schuchardt wrote:
> > 
> > > Kconfig settings that are related to the API for standalone applications
> > > should be in the API sub-menu and not on the top level.
> > > 
> > > CONFIG_STANDALONE_LOAD_ADDR is only relevant if standalone example
> > > applications are built.
> > > 
> > > Signed-off-by: Heinrich Schuchardt 
> > > ---
> > >   Kconfig |  8 
> > >   api/Kconfig | 11 ++-
> > >   2 files changed, 10 insertions(+), 9 deletions(-)
> > 
> > Did you put this through CI? It's possible that some envs don't do
> > "loadaddr=CONFIG_STANDALONE_LOAD_ADDR" and not enable API stuff anymore,
> > but I think that's why I did what I did when migrating.
> > 
> 
> Hello Tom,
> 
> we should keep the main Kconfig menu clean of detail settings. I don't thin
> that there is an issue with the current patch.

Yes, there's many ways Kconfig needs to be cleaned up, especially now
that the migration of symbols from the board.h files is done. Some of
the oddities were a result of symbol misuse/abuse, which can be fixed
now.

> STANDALONE_LOAD_ADDR is not used for loadaddr:
> 
> $ git grep -n STANDALONE_LOAD_ADDR
> (based on origin/master)
> 
> api/Kconfig:15
> config STANDALONE_LOAD_ADDR
> 
> config.mk:79
> export CONFIG_STANDALONE_LOAD_ADDR
> 
> configs/display5_defconfig:33
> CONFIG_STANDALONE_LOAD_ADDR=0x10001000
> 
> configs/display5_factory_defconfig:30
> CONFIG_STANDALONE_LOAD_ADDR=0x10001000
> 
> configs/microchip_mpfs_icicle_defconfig:15
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/qemu-riscv32_defconfig:12
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/qemu-riscv32_smode_defconfig:13
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/qemu-riscv32_spl_defconfig:15
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/qemu-riscv64_defconfig:12
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/qemu-riscv64_smode_defconfig:13
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/qemu-riscv64_spl_defconfig:14
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/sifive_unleashed_defconfig:21
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/sifive_unmatched_defconfig:24
> CONFIG_STANDALONE_LOAD_ADDR=0x8020
> 
> configs/xtfpga_defconfig:12
> CONFIG_STANDALONE_LOAD_ADDR=0x0080
> 
> examples/standalone/Makefile:45
> LDFLAGS_STANDALONE  += -Ttext $(CONFIG_STANDALONE_LOAD_ADDR)
> 
> tools/patman/test_checkpatch.py:208
> CONFIG_STANDALONE_LOAD_ADDR
> 
> With the patch applied
> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15474
> showed no issues.

Thanks for checking. In general, a CI run for making symbols less
visible will make life easier on me when merging.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH] nvedit: simplify do_env_indirect()

2023-03-06 Thread Rasmus Villemoes
Instead of calling env_get(from) up to three times, just do it once,
computing the value we will put into 'to' and error out if that is
NULL (i.e. no 'from' variable and no default provided).

No functional change.

Signed-off-by: Rasmus Villemoes 
---
 cmd/nvedit.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 53e6b57b60..4844eb7f0c 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1026,6 +1026,7 @@ static int do_env_indirect(struct cmd_tbl *cmdtp, int 
flag,
char *from = argv[2];
char *default_value = NULL;
int ret = 0;
+   char *val;
 
if (argc < 3 || argc > 4) {
return CMD_RET_USAGE;
@@ -1035,18 +1036,14 @@ static int do_env_indirect(struct cmd_tbl *cmdtp, int 
flag,
default_value = argv[3];
}
 
-   if (env_get(from) == NULL && default_value == NULL) {
+   val = env_get(from) ?: default_value;
+   if (!val) {
printf("## env indirect: Environment variable for  (%s) 
does not exist.\n", from);
 
return CMD_RET_FAILURE;
}
 
-   if (env_get(from) == NULL) {
-   ret = env_set(to, default_value);
-   }
-   else {
-   ret = env_set(to, env_get(from));
-   }
+   ret = env_set(to, val);
 
if (ret == 0) {
return CMD_RET_SUCCESS;
-- 
2.37.2



Re: [PATCH] ARM: imx: Include on-SoM microSD in list of i.MX6 DHCOM boot devices

2023-03-06 Thread Fabio Estevam

On 05/03/2023 20:21, Marek Vasut wrote:

Add mmc1, which is mapped to optional on-SoM microSD socket,
to the list of distro boot command boot devices.

Signed-off-by: Marek Vasut 


Reviewed-by: Fabio Estevam 


Re: [PATCH] ARM: dts: imx: Add WDT bindings on DH i.MX6 DHSOM

2023-03-06 Thread Fabio Estevam

On 05/03/2023 17:49, Marek Vasut wrote:

Add WDT reboot bindings on DH i.MX6 DHSOM to permit the platform
to reboot via WDT in U-Boot. These are custom U-Boot bindings,
hence they are placed in -u-boot.dtsi .

Signed-off-by: Marek Vasut 


Reviewed-by: Fabio Estevam 


Re: [PATCH] ARM: imx: Convert DH i.MX6 DHSOM to DM_SERIAL

2023-03-06 Thread Fabio Estevam

On 05/03/2023 17:48, Marek Vasut wrote:

Enable CONFIG_DM_SERIAL on DH i.MX6 DHSOM to convert it to DM serial .

Signed-off-by: Marek Vasut 


Reviewed-by: Fabio Estevam 


Re: [PATCH RFC u-boot-mvebu 0/2] arm: mvebu: Fix eMMC boot

2023-03-06 Thread Pali Rohár
On Monday 06 March 2023 11:15:35 Martin Rowe wrote:
> On Sun, 5 Mar 2023 at 16:04, Pali Rohár  wrote:
> 
> > On Sunday 05 March 2023 12:46:34 Pali Rohár wrote:
> > > On Sunday 05 March 2023 02:24:27 Martin Rowe wrote:
> > > > On Sat, 4 Mar 2023 at 10:40, Pali Rohár  wrote:
> > > >
> > > > > Boot configuration stored in EXT_CSC register is completely ignored
> > by
> > > > > BootROM:
> > > > >
> > > > >
> > https://lore.kernel.org/u-boot/CAOAjy5SYPPzWKok-BSGYwZwcKOQt_aZPgh6FTbrFd3F=8dm...@mail.gmail.com/
> > > > >
> > > > > Reflect this eMMC booting in documentation and in the code.
> > > > >
> > > > > Martin, can you test this patch series if SPL and main U-Boot is
> > loaded
> > > > > from the same eMMC HW partition?
> > > > >
> > > >
> > > > boot0: u-boot
> > > >
> > > > Works fine, no issues.
> > > >
> > > >
> > > > boot0: zeroed
> > > > boot1: u-boot
> > > > user: zeroed
> > > >
> > > > It succeeds, eventually...
> > > > ==
> > > > BootROM - 1.73
> > > >
> > > > Booting from MMC
> > > > BootROM: Bad header at offset 
> > > > BootROM: Bad header at offset 0020
> > > > Switching BootPartitions.
> > > >
> > > > U-Boot SPL 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45
> > > > +1000)
> > > > High speed PHY - Version: 2.0
> > > > EEPROM TLV detection failed: Using static config for Clearfog Pro.
> > > > Detected Device ID 6828
> > > > board SerDes lanes topology details:
> > > >  | Lane # | Speed |  Type   |
> > > >  
> > > >  |   0|   3   | SATA0   |
> > > >  |   1|   0   | SGMII1  |
> > > >  |   2|   5   | PCIe1   |
> > > >  |   3|   5   | USB3 HOST1  |
> > > >  |   4|   5   | PCIe2   |
> > > >  |   5|   0   | SGMII2  |
> > > >  
> > > > High speed PHY - Ended Successfully
> > > > mv_ddr: 14.0.0
> > > > DDR3 Training Sequence - Switching XBAR Window to FastPath Window
> > > > mv_ddr: completed successfully
> > > > Trying to boot from MMC1
> > > > ERROR: Invalid kwbimage v1
> > > > mmc_load_image_raw_sector: mmc block read error
> > > > spl: mmc: wrong boot mode
> > > > Trying to boot from BOOTROM
> > > > Returning to BootROM (return address 0x05c4)...
> > > > Timeout waiting card ready
> > > > BootROM: Image checksum verification PASSED
> > > >
> > > >
> > > > U-Boot 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45
> > +1000)
> > > >
> > > > SoC:   MV88F6828-A0 at 1600 MHz
> > > > DRAM:  1 GiB (800 MHz, 32-bit, ECC not enabled)
> > > > Core:  38 devices, 22 uclasses, devicetree: separate
> > > > MMC:   mv_sdh: 0
> > > > Loading Environment from MMC... *** Warning - bad CRC, using default
> > > > environment
> > > >
> > > > Model: SolidRun Clearfog A1
> > > > Board: SolidRun Clearfog Pro
> > > > Net:
> > > > Warning: ethernet@7 (eth1) using random MAC address -
> > 12:07:8b:f9:7a:6f
> > > > eth1: ethernet@7
> > > > Warning: ethernet@3 (eth2) using random MAC address -
> > ee:ed:f3:bb:c2:af
> > > > , eth2: ethernet@3
> > > > Warning: ethernet@34000 (eth3) using random MAC address -
> > ae:34:b9:bb:28:c6
> > > > , eth3: ethernet@34000
> > > > Hit any key to stop autoboot:  0
> > > > =>
> > > > ==
> > > >
> > > > Between "Returning to BootROM" and "Timeout waiting card ready" takes
> > > > around 315 seconds. That's long enough that I thought it had hung
> > > > completely and I only noticed it continue because I left it running
> > while
> > > > working on something else. I tried several things to reduce this
> > timeout,
> > > > including reverting to the "non-removable" dts for shdci, but nothing
> > seems
> > > > to affect it.
> > >
> > > Ok. So now it is in the state that it is working but is slow. Better
> > > than nothing.
> > >
> > > Message "Returning to BootROM" is printed by SPL and message
> > > "Timeout waiting card ready" is printed by BootROM. After printing
> > > "Returning to BootROM" is SPL jumping back to the BootROM so the delay
> > > is for sure in the BootROM. So seems that SPL reconfigures eMMC into
> > > state in which BootROM cannot work with it. Something timeouts, BootROM
> > > reconfigure/retry it and then it work again. It would be needed to
> > > investigate what is happening here. My guess is that this could have
> > > something with eMMC HW partition access, and code for switching
> > > partitions near SPL MMCSD_MODE_EMMCBOOT.
> >
> > Try this change?
> >
> > diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> > index b20eac3dcd38..eb59c41a824e 100644
> > --- a/arch/arm/mach-mvebu/spl.c
> > +++ b/arch/arm/mach-mvebu/spl.c
> > @@ -11,6 +11,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -297,11 +298,33 @@ u32 spl_boot_device(void)
> >
> >  #endif
> >
> > +void restore_emmc_boot_part_config(void)
> > +{
> > +#ifdef CONFIG_SPL_MMC
> > +   struct mmc *mmc;
> > +   int 

Please pull u-boot-marvell/master

2023-03-06 Thread Stefan Roese

Hi Tom,

please pull this small Marvell related fix:


- mvebu: Use 4K sector for Thecus N2350 SPI flash (Tony)


Here the Azure build, without any issues:

https://dev.azure.com/sr0718/u-boot/_build/results?buildId=287=results

Thanks,
Stefan

The following changes since commit 33fb2d130e28982b488c2a54978031835ed2aa71:

  Merge tag 'dm-pull-29feb23' of 
https://source.denx.de/u-boot/custodians/u-boot-dm (2023-03-01 16:07:24 
-0500)


are available in the Git repository at:

  g...@source.denx.de:u-boot/custodians/u-boot-marvell.git

for you to fetch changes up to aed49a05c71897f787f54a204bb5bf5e620c81fc:

  arm: mvebu: Use 4K sector for Thecus N2350 SPI flash (2023-03-06 
10:16:07 +0100)



Tony Dinh (1):
  arm: mvebu: Use 4K sector for Thecus N2350 SPI flash

 configs/n2350_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


Re: [PATCH] arm: mvebu: Use 4K sector for Thecus N2350 SPI flash

2023-03-06 Thread Stefan Roese

On 2/17/23 04:34, Tony Dinh wrote:

Since the SPI flash chip mx25l3205d on this board has 4K-sector
capability, enable it for the envs.

Signed-off-by: Tony Dinh 


Applied to u-boot-marvell/master

Thanks,
Stefan


---

  configs/n2350_defconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/n2350_defconfig b/configs/n2350_defconfig
index dcb2c96791..b85ef0dfeb 100644
--- a/configs/n2350_defconfig
+++ b/configs/n2350_defconfig
@@ -14,7 +14,7 @@ CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff
  CONFIG_TARGET_N2350=y
  CONFIG_ENV_SIZE=0x1
  CONFIG_ENV_OFFSET=0x10
-CONFIG_ENV_SECT_SIZE=0x1
+CONFIG_ENV_SECT_SIZE=0x1000
  CONFIG_DEFAULT_DEVICE_TREE="armada-385-thecus-n2350"
  CONFIG_SPL_TEXT_BASE=0x4030
  CONFIG_SYS_PROMPT="N2350 > "


Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH RFC u-boot-mvebu 0/2] arm: mvebu: Fix eMMC boot

2023-03-06 Thread Martin Rowe
On Sun, 5 Mar 2023 at 16:04, Pali Rohár  wrote:

> On Sunday 05 March 2023 12:46:34 Pali Rohár wrote:
> > On Sunday 05 March 2023 02:24:27 Martin Rowe wrote:
> > > On Sat, 4 Mar 2023 at 10:40, Pali Rohár  wrote:
> > >
> > > > Boot configuration stored in EXT_CSC register is completely ignored
> by
> > > > BootROM:
> > > >
> > > >
> https://lore.kernel.org/u-boot/CAOAjy5SYPPzWKok-BSGYwZwcKOQt_aZPgh6FTbrFd3F=8dm...@mail.gmail.com/
> > > >
> > > > Reflect this eMMC booting in documentation and in the code.
> > > >
> > > > Martin, can you test this patch series if SPL and main U-Boot is
> loaded
> > > > from the same eMMC HW partition?
> > > >
> > >
> > > boot0: u-boot
> > >
> > > Works fine, no issues.
> > >
> > >
> > > boot0: zeroed
> > > boot1: u-boot
> > > user: zeroed
> > >
> > > It succeeds, eventually...
> > > ==
> > > BootROM - 1.73
> > >
> > > Booting from MMC
> > > BootROM: Bad header at offset 
> > > BootROM: Bad header at offset 0020
> > > Switching BootPartitions.
> > >
> > > U-Boot SPL 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45
> > > +1000)
> > > High speed PHY - Version: 2.0
> > > EEPROM TLV detection failed: Using static config for Clearfog Pro.
> > > Detected Device ID 6828
> > > board SerDes lanes topology details:
> > >  | Lane # | Speed |  Type   |
> > >  
> > >  |   0|   3   | SATA0   |
> > >  |   1|   0   | SGMII1  |
> > >  |   2|   5   | PCIe1   |
> > >  |   3|   5   | USB3 HOST1  |
> > >  |   4|   5   | PCIe2   |
> > >  |   5|   0   | SGMII2  |
> > >  
> > > High speed PHY - Ended Successfully
> > > mv_ddr: 14.0.0
> > > DDR3 Training Sequence - Switching XBAR Window to FastPath Window
> > > mv_ddr: completed successfully
> > > Trying to boot from MMC1
> > > ERROR: Invalid kwbimage v1
> > > mmc_load_image_raw_sector: mmc block read error
> > > spl: mmc: wrong boot mode
> > > Trying to boot from BOOTROM
> > > Returning to BootROM (return address 0x05c4)...
> > > Timeout waiting card ready
> > > BootROM: Image checksum verification PASSED
> > >
> > >
> > > U-Boot 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45
> +1000)
> > >
> > > SoC:   MV88F6828-A0 at 1600 MHz
> > > DRAM:  1 GiB (800 MHz, 32-bit, ECC not enabled)
> > > Core:  38 devices, 22 uclasses, devicetree: separate
> > > MMC:   mv_sdh: 0
> > > Loading Environment from MMC... *** Warning - bad CRC, using default
> > > environment
> > >
> > > Model: SolidRun Clearfog A1
> > > Board: SolidRun Clearfog Pro
> > > Net:
> > > Warning: ethernet@7 (eth1) using random MAC address -
> 12:07:8b:f9:7a:6f
> > > eth1: ethernet@7
> > > Warning: ethernet@3 (eth2) using random MAC address -
> ee:ed:f3:bb:c2:af
> > > , eth2: ethernet@3
> > > Warning: ethernet@34000 (eth3) using random MAC address -
> ae:34:b9:bb:28:c6
> > > , eth3: ethernet@34000
> > > Hit any key to stop autoboot:  0
> > > =>
> > > ==
> > >
> > > Between "Returning to BootROM" and "Timeout waiting card ready" takes
> > > around 315 seconds. That's long enough that I thought it had hung
> > > completely and I only noticed it continue because I left it running
> while
> > > working on something else. I tried several things to reduce this
> timeout,
> > > including reverting to the "non-removable" dts for shdci, but nothing
> seems
> > > to affect it.
> >
> > Ok. So now it is in the state that it is working but is slow. Better
> > than nothing.
> >
> > Message "Returning to BootROM" is printed by SPL and message
> > "Timeout waiting card ready" is printed by BootROM. After printing
> > "Returning to BootROM" is SPL jumping back to the BootROM so the delay
> > is for sure in the BootROM. So seems that SPL reconfigures eMMC into
> > state in which BootROM cannot work with it. Something timeouts, BootROM
> > reconfigure/retry it and then it work again. It would be needed to
> > investigate what is happening here. My guess is that this could have
> > something with eMMC HW partition access, and code for switching
> > partitions near SPL MMCSD_MODE_EMMCBOOT.
>
> Try this change?
>
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index b20eac3dcd38..eb59c41a824e 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -297,11 +298,33 @@ u32 spl_boot_device(void)
>
>  #endif
>
> +void restore_emmc_boot_part_config(void)
> +{
> +#ifdef CONFIG_SPL_MMC
> +   struct mmc *mmc;
> +   int ret;
> +
> +   mmc = find_mmc_device(0);
> +   if (!mmc || !mmc->has_init || mmc->part_config ==
> MMCPART_NOAVAILABLE)
> +   return;
> +
> +   ret = mmc_set_part_conf(mmc,
> +   EXT_CSD_EXTRACT_BOOT_ACK(mmc->part_config),
> +   EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config),
> +  

Re: [PATCH 1/1] api: move API related config options into submenu

2023-03-06 Thread Heinrich Schuchardt

On 3/4/23 16:32, Tom Rini wrote:

On Fri, Mar 03, 2023 at 11:31:22PM +0100, Heinrich Schuchardt wrote:


Kconfig settings that are related to the API for standalone applications
should be in the API sub-menu and not on the top level.

CONFIG_STANDALONE_LOAD_ADDR is only relevant if standalone example
applications are built.

Signed-off-by: Heinrich Schuchardt 
---
  Kconfig |  8 
  api/Kconfig | 11 ++-
  2 files changed, 10 insertions(+), 9 deletions(-)


Did you put this through CI? It's possible that some envs don't do
"loadaddr=CONFIG_STANDALONE_LOAD_ADDR" and not enable API stuff anymore,
but I think that's why I did what I did when migrating.



Hello Tom,

we should keep the main Kconfig menu clean of detail settings. I don't 
thin that there is an issue with the current patch.


STANDALONE_LOAD_ADDR is not used for loadaddr:

$ git grep -n STANDALONE_LOAD_ADDR
(based on origin/master)

api/Kconfig:15
config STANDALONE_LOAD_ADDR

config.mk:79
export CONFIG_STANDALONE_LOAD_ADDR

configs/display5_defconfig:33
CONFIG_STANDALONE_LOAD_ADDR=0x10001000

configs/display5_factory_defconfig:30
CONFIG_STANDALONE_LOAD_ADDR=0x10001000

configs/microchip_mpfs_icicle_defconfig:15
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/qemu-riscv32_defconfig:12
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/qemu-riscv32_smode_defconfig:13
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/qemu-riscv32_spl_defconfig:15
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/qemu-riscv64_defconfig:12
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/qemu-riscv64_smode_defconfig:13
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/qemu-riscv64_spl_defconfig:14
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/sifive_unleashed_defconfig:21
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/sifive_unmatched_defconfig:24
CONFIG_STANDALONE_LOAD_ADDR=0x8020

configs/xtfpga_defconfig:12
CONFIG_STANDALONE_LOAD_ADDR=0x0080

examples/standalone/Makefile:45
LDFLAGS_STANDALONE  += -Ttext $(CONFIG_STANDALONE_LOAD_ADDR)

tools/patman/test_checkpatch.py:208
CONFIG_STANDALONE_LOAD_ADDR

With the patch applied
https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15474
showed no issues.

Best regards

Heinrich


Re: [PATCH 09/10] efi: Support showing tables

2023-03-06 Thread Heinrich Schuchardt

On 2/26/23 02:33, Simon Glass wrote:

Add a command to display the tables provided by EFI.

Signed-off-by: Simon Glass 
---

  cmd/efi.c | 40 +++-
  doc/usage/cmd/efi.rst | 22 ++
  2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/cmd/efi.c b/cmd/efi.c
index c0384e0db28..86988d9e9e2 100644
--- a/cmd/efi.c
+++ b/cmd/efi.c
@@ -7,10 +7,12 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
  #include 
+#include 
  #include 

  DECLARE_GLOBAL_DATA_PTR;
@@ -273,8 +275,43 @@ done:
return ret ? CMD_RET_FAILURE : 0;
  }

+static int do_efi_tables(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])



We already have function do_efi_show_tables() to print the list of EFI
configuration tables. Please, avoid code duplication.


+{
+   struct efi_system_table *systab;
+   int i;
+
+   if (IS_ENABLED(CONFIG_EFI_APP)) {
+   systab = efi_get_sys_table();
+   if (!systab) {
+   printf("Cannot read system table\n");
+   return CMD_RET_FAILURE;
+   }
+   } else {
+   int size;
+   int ret;
+
+   ret = efi_info_get(EFIET_SYS_TABLE, (void **), );
+   if (ret) {
+   printf("Cannot find EFI system table (err=%d)\n", ret);
+   return CMD_RET_FAILURE;
+   }
+   }
+   for (i = 0; i < systab->nr_tables; i++) {
+   struct efi_configuration_table *tab = >tables[i];
+   char guid_str[37];
+
+   uuid_bin_to_str(tab->guid.b, guid_str, 1);
+   printf("%p  %s  %s\n", tab->table, guid_str,
+  uuid_guid_get_str(tab->guid.b));



do_efi_show_tables() does this with printf("%pUl (%pUs)\n",..).


+   }
+
+   return 0;
+}
+
  static struct cmd_tbl efi_commands[] = {
U_BOOT_CMD_MKENT(mem, 1, 1, do_efi_mem, "", ""),
+   U_BOOT_CMD_MKENT(tables, 1, 1, do_efi_tables, "", ""),
  };

  static int do_efi(struct cmd_tbl *cmdtp, int flag, int argc, char *const 
argv[])
@@ -298,5 +335,6 @@ static int do_efi(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
  U_BOOT_CMD(
efi, 3,  1,  do_efi,
"EFI access",
-   "mem [all]Dump memory information [include boot services]"
+   "mem [all]Dump memory information [include boot services]\n"
+   "tables   Dump tables"
  );
diff --git a/doc/usage/cmd/efi.rst b/doc/usage/cmd/efi.rst
index c029c423879..2424d3bb587 100644
--- a/doc/usage/cmd/efi.rst
+++ b/doc/usage/cmd/efi.rst
@@ -10,6 +10,7 @@ Synopsis
  ::

  efi mem [all]
+efi tables

  Description
  ---
@@ -54,6 +55,14 @@ Attributes
  Shows a code for memory attributes. The key for this is shown below the
  table.

+efi tables
+~~
+
+This shows a list of the EFI tables provided in the system table. These use
+GUIDs so it is not possible in general to show the name of a table. But some
+effort is made to provide a useful table, where the GUID is known by U-Boot.
+
+
  Example
  ---

@@ -195,3 +204,16 @@ Example
   f: uncached, write-coalescing, write-through, write-back
  rf: uncached, write-coalescing, write-through, write-back, needs runtime 
mapping
   1: uncached
+
+
+=> efi tables
+1f8eda98  ee4e5898-3914-4259-9d6e-dc7bd79403cf  EFI_LZMA_COMPRESSED
+1ff2ace0  05ad34ba-6f02-4214-952e-4da0398e2bb9  EFI_DXE_SERVICES
+1f8ea018  7739f24c-93d7-11d4-9a3a-0090273fc14d  EFI_HOB_LIST
+1ff2bac0  4c19049f-4137-4dd3-9c10-8b97a83ffdfa  EFI_MEMORY_TYPE
+1ff2cb10  49152e77-1ada-4764-b7a2-7afefed95e8b  


This is what 'efi tables' / printf("%pUl (%pUs)\n",..). would output for
an unknown table:

8868e871-e4f1-11d3-bc22-0080c73c8881 (8868e871-e4f1-11d3-bc22-0080c73c8881)

Maybe uuid_guid_get_str() should return an empty string in case of an
unknown GUID.


+1f9ac018  060cc026-4c0d-4dda-8f41-595fef00a502  
EFI_MEM_STATUS_CODE_REC
+1f9ab000  eb9d2d31-2d88-11d3-9a16-0090273fc14d  EFI_SMBIOS
+1fb7e000  eb9d2d30-2d88-11d3-9a16-0090273fc14d  EFI_GUID_EFI_ACPI1
+1fb7e014  8868e871-e4f1-11d3-bc22-0080c73c8881  EFI_GUID_EFI_ACPI2
+1e550018  dcfa911d-26eb-469f-a220-38b7dc461220  


Who would care about the address of the table?

Best regards

Heinrich