diff mbox series

[v2,8/8] board: add support for Qualcomm SA8155P-ADP board

Message ID 20240306005230.2638972-9-volodymyr_babchuk@epam.com
State Changes Requested
Delegated to: Caleb Connolly
Headers show
Series Add support for Qualcomm SA8155-ADP board | expand

Commit Message

Volodymyr Babchuk March 6, 2024, 12:53 a.m. UTC
SA8155P Automotive Development Platform is Qualcomm SA8155-based board
for developers. The nice thing that it has unlocked loaders with test
keys support, which means that U-Boot for this platform can be
launched at earlier stages.

This patch adds basic board support with only serial port and
networking operation. I am using U-Boot to ease up Xen porting onto
this board, so I am mostly interesting in booting U-Boot in EL2. But
more conventional setup with Android boot image is supported as well.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---

Changes in v2:
 - Rebased onto qcom-next branch
 - Removed unnecessary files thanks to generic qualcomm board support
 - Enabled CONFIG_REMAKE_ELF (this removes one extra step in the
   readme)

 arch/arm/dts/sa8155p-adp-u-boot.dtsi   | 30 +++++++++
 board/qualcomm/sa8155p-adp/MAINTAINERS |  5 ++
 configs/sa8155p_adp_defconfig          | 35 +++++++++++
 doc/board/qualcomm/index.rst           |  1 +
 doc/board/qualcomm/sa8155p-adp.rst     | 87 ++++++++++++++++++++++++++
 5 files changed, 158 insertions(+)
 create mode 100644 arch/arm/dts/sa8155p-adp-u-boot.dtsi
 create mode 100644 board/qualcomm/sa8155p-adp/MAINTAINERS
 create mode 100644 configs/sa8155p_adp_defconfig
 create mode 100644 doc/board/qualcomm/sa8155p-adp.rst

Comments

Sumit Garg March 6, 2024, 5:52 a.m. UTC | #1
Hi Volodymyr,

On Wed, 6 Mar 2024 at 06:23, Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> SA8155P Automotive Development Platform is Qualcomm SA8155-based board
> for developers. The nice thing that it has unlocked loaders with test
> keys support, which means that U-Boot for this platform can be
> launched at earlier stages.
>
> This patch adds basic board support with only serial port and
> networking operation. I am using U-Boot to ease up Xen porting onto
> this board, so I am mostly interesting in booting U-Boot in EL2. But
> more conventional setup with Android boot image is supported as well.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>
> ---
>
> Changes in v2:
>  - Rebased onto qcom-next branch
>  - Removed unnecessary files thanks to generic qualcomm board support
>  - Enabled CONFIG_REMAKE_ELF (this removes one extra step in the
>    readme)

Thanks for the rebase.

>
>  arch/arm/dts/sa8155p-adp-u-boot.dtsi   | 30 +++++++++
>  board/qualcomm/sa8155p-adp/MAINTAINERS |  5 ++
>  configs/sa8155p_adp_defconfig          | 35 +++++++++++
>  doc/board/qualcomm/index.rst           |  1 +
>  doc/board/qualcomm/sa8155p-adp.rst     | 87 ++++++++++++++++++++++++++
>  5 files changed, 158 insertions(+)
>  create mode 100644 arch/arm/dts/sa8155p-adp-u-boot.dtsi
>  create mode 100644 board/qualcomm/sa8155p-adp/MAINTAINERS
>  create mode 100644 configs/sa8155p_adp_defconfig
>  create mode 100644 doc/board/qualcomm/sa8155p-adp.rst
>
> diff --git a/arch/arm/dts/sa8155p-adp-u-boot.dtsi b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
> new file mode 100644
> index 0000000000..ffbf0933c7
> --- /dev/null
> +++ b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Qualcomm SA8155P-ADP device tree fixups for U-BOot
> + *
> + * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> + * Copyright (c) 2024 EPAM Systems.
> + */
> +
> +/ {
> +       /* Populate memory node with actual memory configuration */
> +       memory@80000000 {
> +               reg = <0x00 0x80000000 0x00 0x39900000>,
> +               <0x02 0x0        0x1  0x7fd00000>,
> +               <0x00 0xC0000000 0x1  0x40000000>;
> +       };
> +};
> +
> +&ethernet {
> +       /* Ethernet driver tries to find reset by name */
> +       reset-names = "emac";

This deserves to be pushed upstream in Linux kernel DT. In the
meantime we can carry it here.

> +};
> +
> +&tlmm {
> +       /* U-Boot pinctrl driver does not understand multiple tiles */
> +       reg = <0x0 0x03000000 0x0 0x1000000>;
> +       /delete-property/ reg-names;

This won't be needed if we can make the tiles offset in the pinctrl
driver compatible:

#define WEST   0x00000000
#define EAST   0x00400000
#define NORTH  0x00800000
#define SOUTH  0x00C00000

> +
> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
> +       /delete-node/ phy-reset-pins;

I suppose this is not needed as phy-reset-pins also configures the pin
as GPIO only.

> +};
> diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
> new file mode 100644
> index 0000000000..03fac84f51
> --- /dev/null
> +++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
> @@ -0,0 +1,5 @@
> +Qualcomm SA8155P Automotive Development Platform
> +M:     Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> +S:     Maintained
> +F:     board/qualcomm/sa8155p-adp/
> +F:     configs/sa8155p-adp_defconfig
> diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
> new file mode 100644
> index 0000000000..b6969767f8
> --- /dev/null
> +++ b/configs/sa8155p_adp_defconfig
> @@ -0,0 +1,35 @@
> +CONFIG_ARM=y
> +CONFIG_SKIP_LOWLEVEL_INIT=y
> +CONFIG_COUNTER_FREQUENCY=19000000
> +CONFIG_POSITION_INDEPENDENT=y
> +CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
> +CONFIG_ARCH_SNAPDRAGON=y
> +CONFIG_TEXT_BASE=0x85710000

Being position independent shouldn't require a hardcoded U-Boot text
base. Can you try if we can get rid of this?

> +CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
> +CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
> +CONFIG_SYS_LOAD_ADDR=0x85710000

Ditto.

> +CONFIG_REMAKE_ELF=y
> +CONFIG_BOOTDELAY=3
> +CONFIG_SYS_CBSIZE=512
> +# CONFIG_DISPLAY_CPUINFO is not set
> +CONFIG_HUSH_PARSER=y
> +CONFIG_OF_UPSTREAM=y
> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> +CONFIG_NET_RANDOM_ETHADDR=y
> +CONFIG_CLK=y
> +CONFIG_CLK_QCOM_SM8150=y
> +CONFIG_MSM_GPIO=y
> +CONFIG_PHY_MICREL=y
> +CONFIG_PHY_MICREL_KSZ90X1=y
> +CONFIG_DM_MDIO=y
> +CONFIG_DM_ETH_PHY=y
> +CONFIG_DWC_ETH_QOS=y
> +CONFIG_DWC_ETH_QOS_QCOM=y
> +CONFIG_PHY=y
> +CONFIG_PINCTRL=y
> +CONFIG_PINCONF=y
> +CONFIG_PINCTRL_QCOM_SM8150=y
> +CONFIG_POWER_DOMAIN=y
> +CONFIG_MSM_GENI_SERIAL=y
> +CONFIG_SPMI_MSM=y
> +CONFIG_LMB_MAX_REGIONS=64

Apart from above, I think this platform should be able to reuse
qcom_defconfig as you can find most of the config options there. Can
you try to reuse it?

> diff --git a/doc/board/qualcomm/index.rst b/doc/board/qualcomm/index.rst
> index 4955274a39..268218b05f 100644
> --- a/doc/board/qualcomm/index.rst
> +++ b/doc/board/qualcomm/index.rst
> @@ -7,5 +7,6 @@ Qualcomm
>     :maxdepth: 2
>
>     dragonboard410c
> +   sa8155p-adp
>     board
>     debugging
> diff --git a/doc/board/qualcomm/sa8155p-adp.rst b/doc/board/qualcomm/sa8155p-adp.rst
> new file mode 100644
> index 0000000000..66db512b52
> --- /dev/null
> +++ b/doc/board/qualcomm/sa8155p-adp.rst
> @@ -0,0 +1,87 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> +.. sectionauthor:: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> +
> +SA8155P Automotive Development Platform
> +=======================================
> +
> +About
> +-----
> +This document describes the information about SA8155P Automotive
> +Development Platform aka SA8155P-ADP.
> +
> +Currently U-Boot can be booted either as Android boot image, or in EL2
> +mode, instead of hypervisor image. In the latter case it is possible
> +to use U-Boot to either boot Linux with KVM support or to boot Xen
> +Hypervisor on this board.
> +
> +Supported HW modules
> +^^^^^^^^^^^^^^^^^^^^
> +Port for this board is in early development state. Right now U-Boot
> +supports serial console and networking. No USB/fastboot or UFS support
> +yet. So it is not possible to save environment variables as
> +well. Nevertheless this is enough for development as user can download
> +all required images via TFTP.
> +
> +Installation
> +------------
> +Build
> +^^^^^
> +Setup ``CROSS_COMPILE`` for aarch64 and build U-Boot for your board::
> +
> +       $ export CROSS_COMPILE=<aarch64 toolchain prefix>
> +       $ make sa8155p_adp_defconfig
> +       $ make
> +
> +This will build ``u-boot.bin`` in the configured output directory.
> +
> +Boot in EL1 mode instead of Android boot image
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Create a dummy ramdisk image:::
> +
> +       $ echo "This is not a ramdisk" > ramdisk.img
> +
> +Compress u-boot binary:::
> +
> +       $ gzip -c u-boot.bin > u-boot.bin.gz
> +
> +Append DTB again (binary we use already have DTB embedded in, but
> +Android boot image format requires another DTB at the end of the
> +archive):::
> +
> +       $ cat u-boot.bin.gz u-boot.dtb > u-boot.bin.gz-dtb
> +
> +Now we've got everything to build android boot image:::
> +
> +       $ mkbootimg --kernel u-boot.bin.gz-dtb \
> +       --ramdisk ramdisk.img --pagesize 4096 \
> +       --base 0x80000000 -o boot.img
> +
> +Finally you can flash new boot image with fastboot:::
> +
> +       $ fastboot flash boot boot.img
> +
> +Or just boot U-Boot without flashing anything:::
> +
> +       $ fastboot boot boot.img
> +
> +Boot in EL2 mode instead of Qualcomm's hypervisor stub
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This approach ensures that U-Boot is booted in EL2 and it is possible
> +to run virtualization software (like Xen or KVM) on the board. You
> +must understand that this approach breaks Qualcomm's boot chain. You
> +will not be able to call all subsequent loaders, so you will not be
> +able to use fastboot for example. Use this approach only if you want
> +to experiment with virtualization on SA8155P-ADP.
> +
> +U-Boot ELF file needs to be signed with test keys. `qtestsign
> +<https://github.com/msm8916-mainline/qtestsign>`_ tool can be used ::
> +
> +       $ ../qtestsign/qtestsign.py -v6 hyp u-boot.elf
> +
> +Resulting ``u-boot-test-signed.mbn`` then can be written to the
> +board. Easiest way is to use ``edl`` tool: ::
> +
> +       $ ../edl/edl w hyp_a u-boot-test-signed.mbn --memory=ufs --lun=4
> +

Can you provide reference to the EDL tool and its usage so that people
can recover their board if it gets bricked?

-Sumit

> +Be sure to backup existing hyp_a loader before flashing U-Boot.
> --
> 2.43.0
Caleb Connolly March 6, 2024, 2:29 p.m. UTC | #2
On 06/03/2024 00:53, Volodymyr Babchuk wrote:
> SA8155P Automotive Development Platform is Qualcomm SA8155-based board
> for developers. The nice thing that it has unlocked loaders with test
> keys support, which means that U-Boot for this platform can be
> launched at earlier stages.
> 
> This patch adds basic board support with only serial port and
> networking operation. I am using U-Boot to ease up Xen porting onto
> this board, so I am mostly interesting in booting U-Boot in EL2. But
> more conventional setup with Android boot image is supported as well.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> ---
> 
> Changes in v2:
>  - Rebased onto qcom-next branch
>  - Removed unnecessary files thanks to generic qualcomm board support
>  - Enabled CONFIG_REMAKE_ELF (this removes one extra step in the
>    readme)
> 
>  arch/arm/dts/sa8155p-adp-u-boot.dtsi   | 30 +++++++++
>  board/qualcomm/sa8155p-adp/MAINTAINERS |  5 ++
>  configs/sa8155p_adp_defconfig          | 35 +++++++++++
>  doc/board/qualcomm/index.rst           |  1 +
>  doc/board/qualcomm/sa8155p-adp.rst     | 87 ++++++++++++++++++++++++++
>  5 files changed, 158 insertions(+)
>  create mode 100644 arch/arm/dts/sa8155p-adp-u-boot.dtsi
>  create mode 100644 board/qualcomm/sa8155p-adp/MAINTAINERS
>  create mode 100644 configs/sa8155p_adp_defconfig
>  create mode 100644 doc/board/qualcomm/sa8155p-adp.rst
> 
> diff --git a/arch/arm/dts/sa8155p-adp-u-boot.dtsi b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
> new file mode 100644
> index 0000000000..ffbf0933c7
> --- /dev/null
> +++ b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Qualcomm SA8155P-ADP device tree fixups for U-BOot
> + *
> + * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> + * Copyright (c) 2024 EPAM Systems.
> + */
> +
> +/ {
> +	/* Populate memory node with actual memory configuration */
> +	memory@80000000 {
> +		reg = <0x00 0x80000000 0x00 0x39900000>,
> +		<0x02 0x0        0x1  0x7fd00000>,
> +		<0x00 0xC0000000 0x1  0x40000000>;
> +	};
> +};
> +
> +&ethernet {
> +	/* Ethernet driver tries to find reset by name */
> +	reset-names = "emac";
> +};
> +
> +&tlmm {
> +	/* U-Boot pinctrl driver does not understand multiple tiles */
> +	reg = <0x0 0x03000000 0x0 0x1000000>;
could you follow the other pinctrl drivers and just define the tile
offsets relative to the first tile specified in DT? Otherwise every
sm8150 board will need this DTS hack. Fixing this in the driver would
also make it possible to use the same DT to boot Linux which is quite
useful.
> +	/delete-property/ reg-names;
There's no need to drop this property, and Linux depends on it.

With those changes:

Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
> +
> +	/* U-Boot ethernet driver wants to drive reset as GPIO */
> +	/delete-node/ phy-reset-pins;
> +};
> diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
> new file mode 100644
> index 0000000000..03fac84f51
> --- /dev/null
> +++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
> @@ -0,0 +1,5 @@
> +Qualcomm SA8155P Automotive Development Platform
> +M:	Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> +S:	Maintained
> +F:	board/qualcomm/sa8155p-adp/
> +F:	configs/sa8155p-adp_defconfig
> diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
> new file mode 100644
> index 0000000000..b6969767f8
> --- /dev/null
> +++ b/configs/sa8155p_adp_defconfig
> @@ -0,0 +1,35 @@
> +CONFIG_ARM=y
> +CONFIG_SKIP_LOWLEVEL_INIT=y
> +CONFIG_COUNTER_FREQUENCY=19000000
> +CONFIG_POSITION_INDEPENDENT=y
> +CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
> +CONFIG_ARCH_SNAPDRAGON=y
> +CONFIG_TEXT_BASE=0x85710000
> +CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
> +CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
> +CONFIG_SYS_LOAD_ADDR=0x85710000
> +CONFIG_REMAKE_ELF=y
> +CONFIG_BOOTDELAY=3
> +CONFIG_SYS_CBSIZE=512
> +# CONFIG_DISPLAY_CPUINFO is not set
> +CONFIG_HUSH_PARSER=y
> +CONFIG_OF_UPSTREAM=y
> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> +CONFIG_NET_RANDOM_ETHADDR=y
> +CONFIG_CLK=y
> +CONFIG_CLK_QCOM_SM8150=y
> +CONFIG_MSM_GPIO=y
> +CONFIG_PHY_MICREL=y
> +CONFIG_PHY_MICREL_KSZ90X1=y
> +CONFIG_DM_MDIO=y
> +CONFIG_DM_ETH_PHY=y
> +CONFIG_DWC_ETH_QOS=y
> +CONFIG_DWC_ETH_QOS_QCOM=y
> +CONFIG_PHY=y
> +CONFIG_PINCTRL=y
> +CONFIG_PINCONF=y
> +CONFIG_PINCTRL_QCOM_SM8150=y
> +CONFIG_POWER_DOMAIN=y
> +CONFIG_MSM_GENI_SERIAL=y
> +CONFIG_SPMI_MSM=y
> +CONFIG_LMB_MAX_REGIONS=64
> diff --git a/doc/board/qualcomm/index.rst b/doc/board/qualcomm/index.rst
> index 4955274a39..268218b05f 100644
> --- a/doc/board/qualcomm/index.rst
> +++ b/doc/board/qualcomm/index.rst
> @@ -7,5 +7,6 @@ Qualcomm
>     :maxdepth: 2
>  
>     dragonboard410c
> +   sa8155p-adp
>     board
>     debugging
> diff --git a/doc/board/qualcomm/sa8155p-adp.rst b/doc/board/qualcomm/sa8155p-adp.rst
> new file mode 100644
> index 0000000000..66db512b52
> --- /dev/null
> +++ b/doc/board/qualcomm/sa8155p-adp.rst
> @@ -0,0 +1,87 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> +.. sectionauthor:: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> +
> +SA8155P Automotive Development Platform
> +=======================================
> +
> +About
> +-----
> +This document describes the information about SA8155P Automotive
> +Development Platform aka SA8155P-ADP.
> +
> +Currently U-Boot can be booted either as Android boot image, or in EL2
> +mode, instead of hypervisor image. In the latter case it is possible
> +to use U-Boot to either boot Linux with KVM support or to boot Xen
> +Hypervisor on this board.
> +
> +Supported HW modules
> +^^^^^^^^^^^^^^^^^^^^
> +Port for this board is in early development state. Right now U-Boot
> +supports serial console and networking. No USB/fastboot or UFS support
> +yet. So it is not possible to save environment variables as
> +well. Nevertheless this is enough for development as user can download
> +all required images via TFTP.
> +
> +Installation
> +------------
> +Build
> +^^^^^
> +Setup ``CROSS_COMPILE`` for aarch64 and build U-Boot for your board::
> +
> +	$ export CROSS_COMPILE=<aarch64 toolchain prefix>
> +	$ make sa8155p_adp_defconfig
> +	$ make
> +
> +This will build ``u-boot.bin`` in the configured output directory.
> +
> +Boot in EL1 mode instead of Android boot image
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Create a dummy ramdisk image:::
> +
> +	$ echo "This is not a ramdisk" > ramdisk.img
> +
> +Compress u-boot binary:::
> +
> +	$ gzip -c u-boot.bin > u-boot.bin.gz
> +
> +Append DTB again (binary we use already have DTB embedded in, but
> +Android boot image format requires another DTB at the end of the
> +archive):::
> +
> +	$ cat u-boot.bin.gz u-boot.dtb > u-boot.bin.gz-dtb
> +
> +Now we've got everything to build android boot image:::
> +
> +	$ mkbootimg --kernel u-boot.bin.gz-dtb \
> +	--ramdisk ramdisk.img --pagesize 4096 \
> +	--base 0x80000000 -o boot.img
> +
> +Finally you can flash new boot image with fastboot:::
> +
> +	$ fastboot flash boot boot.img
> +
> +Or just boot U-Boot without flashing anything:::
> +
> +	$ fastboot boot boot.img
> +
> +Boot in EL2 mode instead of Qualcomm's hypervisor stub
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This approach ensures that U-Boot is booted in EL2 and it is possible
> +to run virtualization software (like Xen or KVM) on the board. You
> +must understand that this approach breaks Qualcomm's boot chain. You
> +will not be able to call all subsequent loaders, so you will not be
> +able to use fastboot for example. Use this approach only if you want
> +to experiment with virtualization on SA8155P-ADP.
> +
> +U-Boot ELF file needs to be signed with test keys. `qtestsign
> +<https://github.com/msm8916-mainline/qtestsign>`_ tool can be used ::
> +
> +	$ ../qtestsign/qtestsign.py -v6 hyp u-boot.elf
> +
> +Resulting ``u-boot-test-signed.mbn`` then can be written to the
> +board. Easiest way is to use ``edl`` tool: ::
> +
> +	$ ../edl/edl w hyp_a u-boot-test-signed.mbn --memory=ufs --lun=4
> +
> +Be sure to backup existing hyp_a loader before flashing U-Boot.
Volodymyr Babchuk March 6, 2024, 7:14 p.m. UTC | #3
Hi Sumit,

Sumit Garg <sumit.garg@linaro.org> writes:

> Hi Volodymyr,
>
> On Wed, 6 Mar 2024 at 06:23, Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>> SA8155P Automotive Development Platform is Qualcomm SA8155-based board
>> for developers. The nice thing that it has unlocked loaders with test
>> keys support, which means that U-Boot for this platform can be
>> launched at earlier stages.
>>
>> This patch adds basic board support with only serial port and
>> networking operation. I am using U-Boot to ease up Xen porting onto
>> this board, so I am mostly interesting in booting U-Boot in EL2. But
>> more conventional setup with Android boot image is supported as well.
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>
>> ---
>>
>> Changes in v2:
>>  - Rebased onto qcom-next branch
>>  - Removed unnecessary files thanks to generic qualcomm board support
>>  - Enabled CONFIG_REMAKE_ELF (this removes one extra step in the
>>    readme)
>
> Thanks for the rebase.

Thank you for the review.

>>
>>  arch/arm/dts/sa8155p-adp-u-boot.dtsi   | 30 +++++++++
>>  board/qualcomm/sa8155p-adp/MAINTAINERS |  5 ++
>>  configs/sa8155p_adp_defconfig          | 35 +++++++++++
>>  doc/board/qualcomm/index.rst           |  1 +
>>  doc/board/qualcomm/sa8155p-adp.rst     | 87 ++++++++++++++++++++++++++
>>  5 files changed, 158 insertions(+)
>>  create mode 100644 arch/arm/dts/sa8155p-adp-u-boot.dtsi
>>  create mode 100644 board/qualcomm/sa8155p-adp/MAINTAINERS
>>  create mode 100644 configs/sa8155p_adp_defconfig
>>  create mode 100644 doc/board/qualcomm/sa8155p-adp.rst
>>
>> diff --git a/arch/arm/dts/sa8155p-adp-u-boot.dtsi b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
>> new file mode 100644
>> index 0000000000..ffbf0933c7
>> --- /dev/null
>> +++ b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
>> @@ -0,0 +1,30 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Qualcomm SA8155P-ADP device tree fixups for U-BOot
>> + *
>> + * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> + * Copyright (c) 2024 EPAM Systems.
>> + */
>> +
>> +/ {
>> +       /* Populate memory node with actual memory configuration */
>> +       memory@80000000 {
>> +               reg = <0x00 0x80000000 0x00 0x39900000>,
>> +               <0x02 0x0        0x1  0x7fd00000>,
>> +               <0x00 0xC0000000 0x1  0x40000000>;
>> +       };
>> +};
>> +
>> +&ethernet {
>> +       /* Ethernet driver tries to find reset by name */
>> +       reset-names = "emac";
>
> This deserves to be pushed upstream in Linux kernel DT. In the
> meantime we can carry it here.
>
>> +};
>> +
>> +&tlmm {
>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>> +       /delete-property/ reg-names;
>
> This won't be needed if we can make the tiles offset in the pinctrl
> driver compatible:
>
> #define WEST   0x00000000
> #define EAST   0x00400000
> #define NORTH  0x00800000
> #define SOUTH  0x00C00000

Hmm, I assume that in this case pinctrl driver should map all the four
tiles independently? Are there guarantees in U-Boot that four separate
memory regions will be mapped into virtual memory with the same relative
positions? Linux clearly don't make such guarantees.

>> +
>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>> +       /delete-node/ phy-reset-pins;
>
> I suppose this is not needed as phy-reset-pins also configures the pin
> as GPIO only.
>
Well, yes. This also puzzles me up, but for some reason it stops working
if I leave this node intact. Looks like I need to look at this deeper
before posting the next version.

>> +};
>> diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
>> new file mode 100644
>> index 0000000000..03fac84f51
>> --- /dev/null
>> +++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
>> @@ -0,0 +1,5 @@
>> +Qualcomm SA8155P Automotive Development Platform
>> +M:     Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> +S:     Maintained
>> +F:     board/qualcomm/sa8155p-adp/
>> +F:     configs/sa8155p-adp_defconfig
>> diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
>> new file mode 100644
>> index 0000000000..b6969767f8
>> --- /dev/null
>> +++ b/configs/sa8155p_adp_defconfig
>> @@ -0,0 +1,35 @@
>> +CONFIG_ARM=y
>> +CONFIG_SKIP_LOWLEVEL_INIT=y
>> +CONFIG_COUNTER_FREQUENCY=19000000
>> +CONFIG_POSITION_INDEPENDENT=y
>> +CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
>> +CONFIG_ARCH_SNAPDRAGON=y
>> +CONFIG_TEXT_BASE=0x85710000
>
> Being position independent shouldn't require a hardcoded U-Boot text
> base. Can you try if we can get rid of this?
>

Well, it is required if we want to load U-Boot instead of hyp.mbn. We
need correct addresses in the ELF file so Qualcomm loader will not
reject it right away.

>> +CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
>> +CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
>> +CONFIG_SYS_LOAD_ADDR=0x85710000
>
> Ditto.
>
>> +CONFIG_REMAKE_ELF=y
>> +CONFIG_BOOTDELAY=3
>> +CONFIG_SYS_CBSIZE=512
>> +# CONFIG_DISPLAY_CPUINFO is not set
>> +CONFIG_HUSH_PARSER=y
>> +CONFIG_OF_UPSTREAM=y
>> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>> +CONFIG_NET_RANDOM_ETHADDR=y
>> +CONFIG_CLK=y
>> +CONFIG_CLK_QCOM_SM8150=y
>> +CONFIG_MSM_GPIO=y
>> +CONFIG_PHY_MICREL=y
>> +CONFIG_PHY_MICREL_KSZ90X1=y
>> +CONFIG_DM_MDIO=y
>> +CONFIG_DM_ETH_PHY=y
>> +CONFIG_DWC_ETH_QOS=y
>> +CONFIG_DWC_ETH_QOS_QCOM=y
>> +CONFIG_PHY=y
>> +CONFIG_PINCTRL=y
>> +CONFIG_PINCONF=y
>> +CONFIG_PINCTRL_QCOM_SM8150=y
>> +CONFIG_POWER_DOMAIN=y
>> +CONFIG_MSM_GENI_SERIAL=y
>> +CONFIG_SPMI_MSM=y
>> +CONFIG_LMB_MAX_REGIONS=64
>
> Apart from above, I think this platform should be able to reuse
> qcom_defconfig as you can find most of the config options there. Can
> you try to reuse it?

Honestly, the whole reason I am porting U-Boot to this platform is
because I want to run Xen on it. And to run Xen, I need to run U-Boot in
EL2. And to do this I need u-boot.elf with "correct" load address and
entry point.

I am planning to publish and upstream Xen patches as well (once I finish
them). And it will be really nice if Xen users will be able use
mainline U-Boot to boot Xen.

>> diff --git a/doc/board/qualcomm/index.rst b/doc/board/qualcomm/index.rst
>> index 4955274a39..268218b05f 100644
>> --- a/doc/board/qualcomm/index.rst
>> +++ b/doc/board/qualcomm/index.rst
>> @@ -7,5 +7,6 @@ Qualcomm
>>     :maxdepth: 2
>>
>>     dragonboard410c
>> +   sa8155p-adp
>>     board
>>     debugging
>> diff --git a/doc/board/qualcomm/sa8155p-adp.rst b/doc/board/qualcomm/sa8155p-adp.rst
>> new file mode 100644
>> index 0000000000..66db512b52
>> --- /dev/null
>> +++ b/doc/board/qualcomm/sa8155p-adp.rst
>> @@ -0,0 +1,87 @@
>> +.. SPDX-License-Identifier: BSD-3-Clause
>> +.. sectionauthor:: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> +
>> +SA8155P Automotive Development Platform
>> +=======================================
>> +
>> +About
>> +-----
>> +This document describes the information about SA8155P Automotive
>> +Development Platform aka SA8155P-ADP.
>> +
>> +Currently U-Boot can be booted either as Android boot image, or in EL2
>> +mode, instead of hypervisor image. In the latter case it is possible
>> +to use U-Boot to either boot Linux with KVM support or to boot Xen
>> +Hypervisor on this board.
>> +
>> +Supported HW modules
>> +^^^^^^^^^^^^^^^^^^^^
>> +Port for this board is in early development state. Right now U-Boot
>> +supports serial console and networking. No USB/fastboot or UFS support
>> +yet. So it is not possible to save environment variables as
>> +well. Nevertheless this is enough for development as user can download
>> +all required images via TFTP.
>> +
>> +Installation
>> +------------
>> +Build
>> +^^^^^
>> +Setup ``CROSS_COMPILE`` for aarch64 and build U-Boot for your board::
>> +
>> +       $ export CROSS_COMPILE=<aarch64 toolchain prefix>
>> +       $ make sa8155p_adp_defconfig
>> +       $ make
>> +
>> +This will build ``u-boot.bin`` in the configured output directory.
>> +
>> +Boot in EL1 mode instead of Android boot image
>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +Create a dummy ramdisk image:::
>> +
>> +       $ echo "This is not a ramdisk" > ramdisk.img
>> +
>> +Compress u-boot binary:::
>> +
>> +       $ gzip -c u-boot.bin > u-boot.bin.gz
>> +
>> +Append DTB again (binary we use already have DTB embedded in, but
>> +Android boot image format requires another DTB at the end of the
>> +archive):::
>> +
>> +       $ cat u-boot.bin.gz u-boot.dtb > u-boot.bin.gz-dtb
>> +
>> +Now we've got everything to build android boot image:::
>> +
>> +       $ mkbootimg --kernel u-boot.bin.gz-dtb \
>> +       --ramdisk ramdisk.img --pagesize 4096 \
>> +       --base 0x80000000 -o boot.img
>> +
>> +Finally you can flash new boot image with fastboot:::
>> +
>> +       $ fastboot flash boot boot.img
>> +
>> +Or just boot U-Boot without flashing anything:::
>> +
>> +       $ fastboot boot boot.img
>> +
>> +Boot in EL2 mode instead of Qualcomm's hypervisor stub
>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +This approach ensures that U-Boot is booted in EL2 and it is possible
>> +to run virtualization software (like Xen or KVM) on the board. You
>> +must understand that this approach breaks Qualcomm's boot chain. You
>> +will not be able to call all subsequent loaders, so you will not be
>> +able to use fastboot for example. Use this approach only if you want
>> +to experiment with virtualization on SA8155P-ADP.
>> +
>> +U-Boot ELF file needs to be signed with test keys. `qtestsign
>> +<https://urldefense.com/v3/__https://github.com/msm8916-mainline/qtestsign__;!!GF_29dbcQIUBPA!0UfvyQldjKBqYC9K5XA-YFiHToTTSqPC1Bik0hYoZFjtfVhrSIOdYKRg45Pu_frCx0I4Shei89pxPWV2V1fqTuquDoA$
>> [github[.]com]>`_ tool can be used ::
>> +
>> +       $ ../qtestsign/qtestsign.py -v6 hyp u-boot.elf
>> +
>> +Resulting ``u-boot-test-signed.mbn`` then can be written to the
>> +board. Easiest way is to use ``edl`` tool: ::
>> +
>> +       $ ../edl/edl w hyp_a u-boot-test-signed.mbn --memory=ufs --lun=4
>> +
>
> Can you provide reference to the EDL tool and its usage so that people
> can recover their board if it gets bricked?

Sure, I'll add the link in the next version.
Caleb Connolly March 6, 2024, 8:26 p.m. UTC | #4
On 06/03/2024 19:14, Volodymyr Babchuk wrote:
> 
> Hi Sumit,
> 
> Sumit Garg <sumit.garg@linaro.org> writes:
> 
>> Hi Volodymyr,
>>
>> On Wed, 6 Mar 2024 at 06:23, Volodymyr Babchuk
>> <Volodymyr_Babchuk@epam.com> wrote:
>>>
>>> SA8155P Automotive Development Platform is Qualcomm SA8155-based board
>>> for developers. The nice thing that it has unlocked loaders with test
>>> keys support, which means that U-Boot for this platform can be
>>> launched at earlier stages.
>>>
>>> This patch adds basic board support with only serial port and
>>> networking operation. I am using U-Boot to ease up Xen porting onto
>>> this board, so I am mostly interesting in booting U-Boot in EL2. But
>>> more conventional setup with Android boot image is supported as well.
>>>
>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>
>>> ---
>>>
>>> Changes in v2:
>>>  - Rebased onto qcom-next branch
>>>  - Removed unnecessary files thanks to generic qualcomm board support
>>>  - Enabled CONFIG_REMAKE_ELF (this removes one extra step in the
>>>    readme)
>>
>> Thanks for the rebase.
> 
> Thank you for the review.
> 
>>>
>>>  arch/arm/dts/sa8155p-adp-u-boot.dtsi   | 30 +++++++++
>>>  board/qualcomm/sa8155p-adp/MAINTAINERS |  5 ++
>>>  configs/sa8155p_adp_defconfig          | 35 +++++++++++
>>>  doc/board/qualcomm/index.rst           |  1 +
>>>  doc/board/qualcomm/sa8155p-adp.rst     | 87 ++++++++++++++++++++++++++
>>>  5 files changed, 158 insertions(+)
>>>  create mode 100644 arch/arm/dts/sa8155p-adp-u-boot.dtsi
>>>  create mode 100644 board/qualcomm/sa8155p-adp/MAINTAINERS
>>>  create mode 100644 configs/sa8155p_adp_defconfig
>>>  create mode 100644 doc/board/qualcomm/sa8155p-adp.rst
>>>
>>> diff --git a/arch/arm/dts/sa8155p-adp-u-boot.dtsi b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
>>> new file mode 100644
>>> index 0000000000..ffbf0933c7
>>> --- /dev/null
>>> +++ b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
>>> @@ -0,0 +1,30 @@
>>> +// SPDX-License-Identifier: BSD-3-Clause
>>> +/*
>>> + * Qualcomm SA8155P-ADP device tree fixups for U-BOot
>>> + *
>>> + * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>> + * Copyright (c) 2024 EPAM Systems.
>>> + */
>>> +
>>> +/ {
>>> +       /* Populate memory node with actual memory configuration */
>>> +       memory@80000000 {
>>> +               reg = <0x00 0x80000000 0x00 0x39900000>,
>>> +               <0x02 0x0        0x1  0x7fd00000>,
>>> +               <0x00 0xC0000000 0x1  0x40000000>;
>>> +       };
>>> +};
>>> +
>>> +&ethernet {
>>> +       /* Ethernet driver tries to find reset by name */
>>> +       reset-names = "emac";
>>
>> This deserves to be pushed upstream in Linux kernel DT. In the
>> meantime we can carry it here.
>>
>>> +};
>>> +
>>> +&tlmm {
>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>> +       /delete-property/ reg-names;
>>
>> This won't be needed if we can make the tiles offset in the pinctrl
>> driver compatible:
>>
>> #define WEST   0x00000000
>> #define EAST   0x00400000
>> #define NORTH  0x00800000
>> #define SOUTH  0x00C00000
> 
> Hmm, I assume that in this case pinctrl driver should map all the four
> tiles independently? Are there guarantees in U-Boot that four separate
> memory regions will be mapped into virtual memory with the same relative
> positions? Linux clearly don't make such guarantees.

U-Boot doesn't use virtual addresses on arm platforms, it only goes as
far as reading the address from DT, nothing else, so this is totally
fine and is how the other SoCs do it.
> 
>>> +
>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>> +       /delete-node/ phy-reset-pins;
>>
>> I suppose this is not needed as phy-reset-pins also configures the pin
>> as GPIO only.
>>
> Well, yes. This also puzzles me up, but for some reason it stops working
> if I leave this node intact. Looks like I need to look at this deeper
> before posting the next version.

Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
misbehave, can you check if this patch fixes it (there is a bug in the
line "return msm_gpio_direction_input(dev, gpio);", it should become
just "msm_gpio_direction_input(dev, gpio);").

I had the exact same issue with the gpio-regulator driver and this was
the solution I ended up going with.

https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/
> 
>>> +};
>>> diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
>>> new file mode 100644
>>> index 0000000000..03fac84f51
>>> --- /dev/null
>>> +++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
>>> @@ -0,0 +1,5 @@
>>> +Qualcomm SA8155P Automotive Development Platform
>>> +M:     Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>> +S:     Maintained
>>> +F:     board/qualcomm/sa8155p-adp/
>>> +F:     configs/sa8155p-adp_defconfig
>>> diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
>>> new file mode 100644
>>> index 0000000000..b6969767f8
>>> --- /dev/null
>>> +++ b/configs/sa8155p_adp_defconfig
>>> @@ -0,0 +1,35 @@
>>> +CONFIG_ARM=y
>>> +CONFIG_SKIP_LOWLEVEL_INIT=y
>>> +CONFIG_COUNTER_FREQUENCY=19000000
>>> +CONFIG_POSITION_INDEPENDENT=y
>>> +CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
>>> +CONFIG_ARCH_SNAPDRAGON=y
>>> +CONFIG_TEXT_BASE=0x85710000
>>
>> Being position independent shouldn't require a hardcoded U-Boot text
>> base. Can you try if we can get rid of this?
>>
> 
> Well, it is required if we want to load U-Boot instead of hyp.mbn. We
> need correct addresses in the ELF file so Qualcomm loader will not
> reject it right away.
> 
>>> +CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
>>> +CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
>>> +CONFIG_SYS_LOAD_ADDR=0x85710000
>>
>> Ditto.
>>
>>> +CONFIG_REMAKE_ELF=y
>>> +CONFIG_BOOTDELAY=3
>>> +CONFIG_SYS_CBSIZE=512
>>> +# CONFIG_DISPLAY_CPUINFO is not set
>>> +CONFIG_HUSH_PARSER=y
>>> +CONFIG_OF_UPSTREAM=y
>>> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>>> +CONFIG_NET_RANDOM_ETHADDR=y
>>> +CONFIG_CLK=y
>>> +CONFIG_CLK_QCOM_SM8150=y
>>> +CONFIG_MSM_GPIO=y
>>> +CONFIG_PHY_MICREL=y
>>> +CONFIG_PHY_MICREL_KSZ90X1=y
>>> +CONFIG_DM_MDIO=y
>>> +CONFIG_DM_ETH_PHY=y
>>> +CONFIG_DWC_ETH_QOS=y
>>> +CONFIG_DWC_ETH_QOS_QCOM=y
>>> +CONFIG_PHY=y
>>> +CONFIG_PINCTRL=y
>>> +CONFIG_PINCONF=y
>>> +CONFIG_PINCTRL_QCOM_SM8150=y
>>> +CONFIG_POWER_DOMAIN=y
>>> +CONFIG_MSM_GENI_SERIAL=y
>>> +CONFIG_SPMI_MSM=y
>>> +CONFIG_LMB_MAX_REGIONS=64
>>
>> Apart from above, I think this platform should be able to reuse
>> qcom_defconfig as you can find most of the config options there. Can
>> you try to reuse it?
> 
> Honestly, the whole reason I am porting U-Boot to this platform is
> because I want to run Xen on it. And to run Xen, I need to run U-Boot in
> EL2. And to do this I need u-boot.elf with "correct" load address and
> entry point.
> 
> I am planning to publish and upstream Xen patches as well (once I finish
> them). And it will be really nice if Xen users will be able use
> mainline U-Boot to boot Xen.

I would like to enable the SM8150 drivers in qcom_defconfig (for
chainloading and supporting other platforms). But I'm totally fine with
having a separate defconfig for this board with this configuration.
> 
>>> diff --git a/doc/board/qualcomm/index.rst b/doc/board/qualcomm/index.rst
>>> index 4955274a39..268218b05f 100644
>>> --- a/doc/board/qualcomm/index.rst
>>> +++ b/doc/board/qualcomm/index.rst
>>> @@ -7,5 +7,6 @@ Qualcomm
>>>     :maxdepth: 2
>>>
>>>     dragonboard410c
>>> +   sa8155p-adp
>>>     board
>>>     debugging
>>> diff --git a/doc/board/qualcomm/sa8155p-adp.rst b/doc/board/qualcomm/sa8155p-adp.rst
>>> new file mode 100644
>>> index 0000000000..66db512b52
>>> --- /dev/null
>>> +++ b/doc/board/qualcomm/sa8155p-adp.rst
>>> @@ -0,0 +1,87 @@
>>> +.. SPDX-License-Identifier: BSD-3-Clause
>>> +.. sectionauthor:: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>> +
>>> +SA8155P Automotive Development Platform
>>> +=======================================
>>> +
>>> +About
>>> +-----
>>> +This document describes the information about SA8155P Automotive
>>> +Development Platform aka SA8155P-ADP.
>>> +
>>> +Currently U-Boot can be booted either as Android boot image, or in EL2
>>> +mode, instead of hypervisor image. In the latter case it is possible
>>> +to use U-Boot to either boot Linux with KVM support or to boot Xen
>>> +Hypervisor on this board.
>>> +
>>> +Supported HW modules
>>> +^^^^^^^^^^^^^^^^^^^^
>>> +Port for this board is in early development state. Right now U-Boot
>>> +supports serial console and networking. No USB/fastboot or UFS support
>>> +yet. So it is not possible to save environment variables as
>>> +well. Nevertheless this is enough for development as user can download
>>> +all required images via TFTP.
>>> +
>>> +Installation
>>> +------------
>>> +Build
>>> +^^^^^
>>> +Setup ``CROSS_COMPILE`` for aarch64 and build U-Boot for your board::
>>> +
>>> +       $ export CROSS_COMPILE=<aarch64 toolchain prefix>
>>> +       $ make sa8155p_adp_defconfig
>>> +       $ make
>>> +
>>> +This will build ``u-boot.bin`` in the configured output directory.
>>> +
>>> +Boot in EL1 mode instead of Android boot image
>>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> +
>>> +Create a dummy ramdisk image:::
>>> +
>>> +       $ echo "This is not a ramdisk" > ramdisk.img
>>> +
>>> +Compress u-boot binary:::
>>> +
>>> +       $ gzip -c u-boot.bin > u-boot.bin.gz
>>> +
>>> +Append DTB again (binary we use already have DTB embedded in, but
>>> +Android boot image format requires another DTB at the end of the
>>> +archive):::
>>> +
>>> +       $ cat u-boot.bin.gz u-boot.dtb > u-boot.bin.gz-dtb
>>> +
>>> +Now we've got everything to build android boot image:::
>>> +
>>> +       $ mkbootimg --kernel u-boot.bin.gz-dtb \
>>> +       --ramdisk ramdisk.img --pagesize 4096 \
>>> +       --base 0x80000000 -o boot.img
>>> +
>>> +Finally you can flash new boot image with fastboot:::
>>> +
>>> +       $ fastboot flash boot boot.img
>>> +
>>> +Or just boot U-Boot without flashing anything:::
>>> +
>>> +       $ fastboot boot boot.img
>>> +
>>> +Boot in EL2 mode instead of Qualcomm's hypervisor stub
>>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> +This approach ensures that U-Boot is booted in EL2 and it is possible
>>> +to run virtualization software (like Xen or KVM) on the board. You
>>> +must understand that this approach breaks Qualcomm's boot chain. You
>>> +will not be able to call all subsequent loaders, so you will not be
>>> +able to use fastboot for example. Use this approach only if you want
>>> +to experiment with virtualization on SA8155P-ADP.
>>> +
>>> +U-Boot ELF file needs to be signed with test keys. `qtestsign
>>> +<https://urldefense.com/v3/__https://github.com/msm8916-mainline/qtestsign__;!!GF_29dbcQIUBPA!0UfvyQldjKBqYC9K5XA-YFiHToTTSqPC1Bik0hYoZFjtfVhrSIOdYKRg45Pu_frCx0I4Shei89pxPWV2V1fqTuquDoA$
>>> [github[.]com]>`_ tool can be used ::
>>> +
>>> +       $ ../qtestsign/qtestsign.py -v6 hyp u-boot.elf
>>> +
>>> +Resulting ``u-boot-test-signed.mbn`` then can be written to the
>>> +board. Easiest way is to use ``edl`` tool: ::
>>> +
>>> +       $ ../edl/edl w hyp_a u-boot-test-signed.mbn --memory=ufs --lun=4
>>> +
>>
>> Can you provide reference to the EDL tool and its usage so that people
>> can recover their board if it gets bricked?
> 
> Sure, I'll add the link in the next version.
>
Volodymyr Babchuk March 6, 2024, 9:24 p.m. UTC | #5
Hi Caleb,

Caleb Connolly <caleb.connolly@linaro.org> writes:

[...]
>>>> +};
>>>> +
>>>> +&tlmm {
>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>> +       /delete-property/ reg-names;
>>>
>>> This won't be needed if we can make the tiles offset in the pinctrl
>>> driver compatible:
>>>
>>> #define WEST   0x00000000
>>> #define EAST   0x00400000
>>> #define NORTH  0x00800000
>>> #define SOUTH  0x00C00000
>> 
>> Hmm, I assume that in this case pinctrl driver should map all the four
>> tiles independently? Are there guarantees in U-Boot that four separate
>> memory regions will be mapped into virtual memory with the same relative
>> positions? Linux clearly don't make such guarantees.
>
> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
> far as reading the address from DT, nothing else, so this is totally
> fine and is how the other SoCs do it.

For me it looks like we are depending on implementation details
knowledge. I.e MMU API does not provide such guarantees, but drivers
know how ARM MMU code is working internally and drivers depend on
exactly this behavior. But if you are saying that it is totally fine,
I'll rework the patch. No big deal. Actually, I already tried this and
it is working fine.

>>>> +
>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>> +       /delete-node/ phy-reset-pins;
>>>
>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>> as GPIO only.
>>>
>> Well, yes. This also puzzles me up, but for some reason it stops working
>> if I leave this node intact. Looks like I need to look at this deeper
>> before posting the next version.
>
> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
> misbehave, can you check if this patch fixes it (there is a bug in the
> line "return msm_gpio_direction_input(dev, gpio);", it should become
> just "msm_gpio_direction_input(dev, gpio);").
>
> I had the exact same issue with the gpio-regulator driver and this was
> the solution I ended up going with.
>
> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
> [lore[.]kernel[.]org]

It is exactly this. With your patch I don't need to /delete-node/
anymore. I'll add a comment in the cover message that this series are
depended on your patch.

(and sorry for the mangled link. It is our corporate mail server doing)


>> 
>>>> +};
>>>> diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
>>>> new file mode 100644
>>>> index 0000000000..03fac84f51
>>>> --- /dev/null
>>>> +++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
>>>> @@ -0,0 +1,5 @@
>>>> +Qualcomm SA8155P Automotive Development Platform
>>>> +M:     Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>> +S:     Maintained
>>>> +F:     board/qualcomm/sa8155p-adp/
>>>> +F:     configs/sa8155p-adp_defconfig
>>>> diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
>>>> new file mode 100644
>>>> index 0000000000..b6969767f8
>>>> --- /dev/null
>>>> +++ b/configs/sa8155p_adp_defconfig
>>>> @@ -0,0 +1,35 @@
>>>> +CONFIG_ARM=y
>>>> +CONFIG_SKIP_LOWLEVEL_INIT=y
>>>> +CONFIG_COUNTER_FREQUENCY=19000000
>>>> +CONFIG_POSITION_INDEPENDENT=y
>>>> +CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
>>>> +CONFIG_ARCH_SNAPDRAGON=y
>>>> +CONFIG_TEXT_BASE=0x85710000
>>>
>>> Being position independent shouldn't require a hardcoded U-Boot text
>>> base. Can you try if we can get rid of this?
>>>
>> 
>> Well, it is required if we want to load U-Boot instead of hyp.mbn. We
>> need correct addresses in the ELF file so Qualcomm loader will not
>> reject it right away.
>> 
>>>> +CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
>>>> +CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
>>>> +CONFIG_SYS_LOAD_ADDR=0x85710000
>>>
>>> Ditto.
>>>
>>>> +CONFIG_REMAKE_ELF=y
>>>> +CONFIG_BOOTDELAY=3
>>>> +CONFIG_SYS_CBSIZE=512
>>>> +# CONFIG_DISPLAY_CPUINFO is not set
>>>> +CONFIG_HUSH_PARSER=y
>>>> +CONFIG_OF_UPSTREAM=y
>>>> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>>>> +CONFIG_NET_RANDOM_ETHADDR=y
>>>> +CONFIG_CLK=y
>>>> +CONFIG_CLK_QCOM_SM8150=y
>>>> +CONFIG_MSM_GPIO=y
>>>> +CONFIG_PHY_MICREL=y
>>>> +CONFIG_PHY_MICREL_KSZ90X1=y
>>>> +CONFIG_DM_MDIO=y
>>>> +CONFIG_DM_ETH_PHY=y
>>>> +CONFIG_DWC_ETH_QOS=y
>>>> +CONFIG_DWC_ETH_QOS_QCOM=y
>>>> +CONFIG_PHY=y
>>>> +CONFIG_PINCTRL=y
>>>> +CONFIG_PINCONF=y
>>>> +CONFIG_PINCTRL_QCOM_SM8150=y
>>>> +CONFIG_POWER_DOMAIN=y
>>>> +CONFIG_MSM_GENI_SERIAL=y
>>>> +CONFIG_SPMI_MSM=y
>>>> +CONFIG_LMB_MAX_REGIONS=64
>>>
>>> Apart from above, I think this platform should be able to reuse
>>> qcom_defconfig as you can find most of the config options there. Can
>>> you try to reuse it?
>> 
>> Honestly, the whole reason I am porting U-Boot to this platform is
>> because I want to run Xen on it. And to run Xen, I need to run U-Boot in
>> EL2. And to do this I need u-boot.elf with "correct" load address and
>> entry point.
>> 
>> I am planning to publish and upstream Xen patches as well (once I finish
>> them). And it will be really nice if Xen users will be able use
>> mainline U-Boot to boot Xen.
>
> I would like to enable the SM8150 drivers in qcom_defconfig (for
> chainloading and supporting other platforms). But I'm totally fine with
> having a separate defconfig for this board with this configuration.

Yes, this is a good approach. I'll do this.

[...]
Sumit Garg March 7, 2024, 7:56 a.m. UTC | #6
On Thu, 7 Mar 2024 at 02:54, Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
>
> Hi Caleb,
>
> Caleb Connolly <caleb.connolly@linaro.org> writes:
>
> [...]
> >>>> +};
> >>>> +
> >>>> +&tlmm {
> >>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
> >>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
> >>>> +       /delete-property/ reg-names;
> >>>
> >>> This won't be needed if we can make the tiles offset in the pinctrl
> >>> driver compatible:
> >>>
> >>> #define WEST   0x00000000
> >>> #define EAST   0x00400000
> >>> #define NORTH  0x00800000
> >>> #define SOUTH  0x00C00000
> >>
> >> Hmm, I assume that in this case pinctrl driver should map all the four
> >> tiles independently? Are there guarantees in U-Boot that four separate
> >> memory regions will be mapped into virtual memory with the same relative
> >> positions? Linux clearly don't make such guarantees.
> >
> > U-Boot doesn't use virtual addresses on arm platforms, it only goes as
> > far as reading the address from DT, nothing else, so this is totally
> > fine and is how the other SoCs do it.
>
> For me it looks like we are depending on implementation details
> knowledge. I.e MMU API does not provide such guarantees, but drivers
> know how ARM MMU code is working internally and drivers depend on
> exactly this behavior. But if you are saying that it is totally fine,
> I'll rework the patch. No big deal. Actually, I already tried this and
> it is working fine.

I agree doing it properly via mapping pinctrl tiles individually is
the appropriate approach. BTW, the current implementation just relies
on 1:1 VA to PA mapping in U-Boot, it just avoids deviation from
upstream DTS. If you are willing to change msm_pinctrl_probe() to
parse tiles properly then I would be happy to review that.

>
> >>>> +
> >>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
> >>>> +       /delete-node/ phy-reset-pins;
> >>>
> >>> I suppose this is not needed as phy-reset-pins also configures the pin
> >>> as GPIO only.
> >>>
> >> Well, yes. This also puzzles me up, but for some reason it stops working
> >> if I leave this node intact. Looks like I need to look at this deeper
> >> before posting the next version.
> >
> > Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
> > misbehave, can you check if this patch fixes it (there is a bug in the
> > line "return msm_gpio_direction_input(dev, gpio);", it should become
> > just "msm_gpio_direction_input(dev, gpio);").
> >
> > I had the exact same issue with the gpio-regulator driver and this was
> > the solution I ended up going with.
> >
> > https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
> > [lore[.]kernel[.]org]
>
> It is exactly this. With your patch I don't need to /delete-node/
> anymore. I'll add a comment in the cover message that this series are
> depended on your patch.
>

+1

> (and sorry for the mangled link. It is our corporate mail server doing)
>
>
> >>
> >>>> +};
> >>>> diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
> >>>> new file mode 100644
> >>>> index 0000000000..03fac84f51
> >>>> --- /dev/null
> >>>> +++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
> >>>> @@ -0,0 +1,5 @@
> >>>> +Qualcomm SA8155P Automotive Development Platform
> >>>> +M:     Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> >>>> +S:     Maintained
> >>>> +F:     board/qualcomm/sa8155p-adp/
> >>>> +F:     configs/sa8155p-adp_defconfig
> >>>> diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
> >>>> new file mode 100644
> >>>> index 0000000000..b6969767f8
> >>>> --- /dev/null
> >>>> +++ b/configs/sa8155p_adp_defconfig
> >>>> @@ -0,0 +1,35 @@
> >>>> +CONFIG_ARM=y
> >>>> +CONFIG_SKIP_LOWLEVEL_INIT=y
> >>>> +CONFIG_COUNTER_FREQUENCY=19000000
> >>>> +CONFIG_POSITION_INDEPENDENT=y
> >>>> +CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
> >>>> +CONFIG_ARCH_SNAPDRAGON=y
> >>>> +CONFIG_TEXT_BASE=0x85710000
> >>>
> >>> Being position independent shouldn't require a hardcoded U-Boot text
> >>> base. Can you try if we can get rid of this?
> >>>
> >>
> >> Well, it is required if we want to load U-Boot instead of hyp.mbn. We
> >> need correct addresses in the ELF file so Qualcomm loader will not
> >> reject it right away.
> >>

I see the hard dependency due to prior stage Qcom bootloaders.

> >>>> +CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
> >>>> +CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
> >>>> +CONFIG_SYS_LOAD_ADDR=0x85710000
> >>>
> >>> Ditto.
> >>>
> >>>> +CONFIG_REMAKE_ELF=y
> >>>> +CONFIG_BOOTDELAY=3
> >>>> +CONFIG_SYS_CBSIZE=512
> >>>> +# CONFIG_DISPLAY_CPUINFO is not set
> >>>> +CONFIG_HUSH_PARSER=y
> >>>> +CONFIG_OF_UPSTREAM=y
> >>>> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >>>> +CONFIG_NET_RANDOM_ETHADDR=y
> >>>> +CONFIG_CLK=y
> >>>> +CONFIG_CLK_QCOM_SM8150=y
> >>>> +CONFIG_MSM_GPIO=y
> >>>> +CONFIG_PHY_MICREL=y
> >>>> +CONFIG_PHY_MICREL_KSZ90X1=y
> >>>> +CONFIG_DM_MDIO=y
> >>>> +CONFIG_DM_ETH_PHY=y
> >>>> +CONFIG_DWC_ETH_QOS=y
> >>>> +CONFIG_DWC_ETH_QOS_QCOM=y
> >>>> +CONFIG_PHY=y
> >>>> +CONFIG_PINCTRL=y
> >>>> +CONFIG_PINCONF=y
> >>>> +CONFIG_PINCTRL_QCOM_SM8150=y
> >>>> +CONFIG_POWER_DOMAIN=y
> >>>> +CONFIG_MSM_GENI_SERIAL=y
> >>>> +CONFIG_SPMI_MSM=y
> >>>> +CONFIG_LMB_MAX_REGIONS=64
> >>>
> >>> Apart from above, I think this platform should be able to reuse
> >>> qcom_defconfig as you can find most of the config options there. Can
> >>> you try to reuse it?
> >>
> >> Honestly, the whole reason I am porting U-Boot to this platform is
> >> because I want to run Xen on it. And to run Xen, I need to run U-Boot in
> >> EL2. And to do this I need u-boot.elf with "correct" load address and
> >> entry point.
> >>
> >> I am planning to publish and upstream Xen patches as well (once I finish
> >> them). And it will be really nice if Xen users will be able use
> >> mainline U-Boot to boot Xen.

That's fair and I am happy for it to be a separate defconfig.

> >
> > I would like to enable the SM8150 drivers in qcom_defconfig (for
> > chainloading and supporting other platforms). But I'm totally fine with
> > having a separate defconfig for this board with this configuration.

Probably the common defconfig is up for renaming:
s/qcom_defconfig/qcom_chainload_defconfig/ since that seems to be the
only configuration we can support with a common defconfig.

>
> Yes, this is a good approach. I'll do this.
>

So you can probably drop CONFIG_POSITION_INDEPENDENT=y from this
defconfig and rather use the common defconfig for chainloaded
configuration.

-Sumit

> [...]


>
> --
> WBR, Volodymyr
Caleb Connolly March 11, 2024, 3:13 p.m. UTC | #7
On 06/03/2024 21:24, Volodymyr Babchuk wrote:
> 
> Hi Caleb,
> 
> Caleb Connolly <caleb.connolly@linaro.org> writes:
> 
> [...]
>>>>> +};
>>>>> +
>>>>> +&tlmm {
>>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>>> +       /delete-property/ reg-names;
>>>>
>>>> This won't be needed if we can make the tiles offset in the pinctrl
>>>> driver compatible:
>>>>
>>>> #define WEST   0x00000000
>>>> #define EAST   0x00400000
>>>> #define NORTH  0x00800000
>>>> #define SOUTH  0x00C00000
>>>
>>> Hmm, I assume that in this case pinctrl driver should map all the four
>>> tiles independently? Are there guarantees in U-Boot that four separate
>>> memory regions will be mapped into virtual memory with the same relative
>>> positions? Linux clearly don't make such guarantees.
>>
>> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
>> far as reading the address from DT, nothing else, so this is totally
>> fine and is how the other SoCs do it.
> 
> For me it looks like we are depending on implementation details
> knowledge. I.e MMU API does not provide such guarantees, but drivers
> know how ARM MMU code is working internally and drivers depend on
> exactly this behavior. But if you are saying that it is totally fine,
> I'll rework the patch. No big deal. Actually, I already tried this and
> it is working fine.
> 
>>>>> +
>>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>>> +       /delete-node/ phy-reset-pins;
>>>>
>>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>>> as GPIO only.
>>>>
>>> Well, yes. This also puzzles me up, but for some reason it stops working
>>> if I leave this node intact. Looks like I need to look at this deeper
>>> before posting the next version.
>>
>> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
>> misbehave, can you check if this patch fixes it (there is a bug in the
>> line "return msm_gpio_direction_input(dev, gpio);", it should become
>> just "msm_gpio_direction_input(dev, gpio);").
>>
>> I had the exact same issue with the gpio-regulator driver and this was
>> the solution I ended up going with.
>>
>> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
>> [lore[.]kernel[.]org]
> 
> It is exactly this. With your patch I don't need to /delete-node/
> anymore. I'll add a comment in the cover message that this series are
> depended on your patch.

Please can you split the power domain and clock patches into a separate
series? As I'd like to depend on them for the next revision of my
series, and we'd otherwise have a cyclical dependency.
> 
> (and sorry for the mangled link. It is our corporate mail server doing)
> 
> 
>>>
>>>>> +};
>>>>> diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
>>>>> new file mode 100644
>>>>> index 0000000000..03fac84f51
>>>>> --- /dev/null
>>>>> +++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
>>>>> @@ -0,0 +1,5 @@
>>>>> +Qualcomm SA8155P Automotive Development Platform
>>>>> +M:     Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>>> +S:     Maintained
>>>>> +F:     board/qualcomm/sa8155p-adp/
>>>>> +F:     configs/sa8155p-adp_defconfig
>>>>> diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
>>>>> new file mode 100644
>>>>> index 0000000000..b6969767f8
>>>>> --- /dev/null
>>>>> +++ b/configs/sa8155p_adp_defconfig
>>>>> @@ -0,0 +1,35 @@
>>>>> +CONFIG_ARM=y
>>>>> +CONFIG_SKIP_LOWLEVEL_INIT=y
>>>>> +CONFIG_COUNTER_FREQUENCY=19000000
>>>>> +CONFIG_POSITION_INDEPENDENT=y
>>>>> +CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
>>>>> +CONFIG_ARCH_SNAPDRAGON=y
>>>>> +CONFIG_TEXT_BASE=0x85710000
>>>>
>>>> Being position independent shouldn't require a hardcoded U-Boot text
>>>> base. Can you try if we can get rid of this?
>>>>
>>>
>>> Well, it is required if we want to load U-Boot instead of hyp.mbn. We
>>> need correct addresses in the ELF file so Qualcomm loader will not
>>> reject it right away.
>>>
>>>>> +CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
>>>>> +CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
>>>>> +CONFIG_SYS_LOAD_ADDR=0x85710000
>>>>
>>>> Ditto.
>>>>
>>>>> +CONFIG_REMAKE_ELF=y
>>>>> +CONFIG_BOOTDELAY=3
>>>>> +CONFIG_SYS_CBSIZE=512
>>>>> +# CONFIG_DISPLAY_CPUINFO is not set
>>>>> +CONFIG_HUSH_PARSER=y
>>>>> +CONFIG_OF_UPSTREAM=y
>>>>> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>>>>> +CONFIG_NET_RANDOM_ETHADDR=y
>>>>> +CONFIG_CLK=y
>>>>> +CONFIG_CLK_QCOM_SM8150=y
>>>>> +CONFIG_MSM_GPIO=y
>>>>> +CONFIG_PHY_MICREL=y
>>>>> +CONFIG_PHY_MICREL_KSZ90X1=y
>>>>> +CONFIG_DM_MDIO=y
>>>>> +CONFIG_DM_ETH_PHY=y
>>>>> +CONFIG_DWC_ETH_QOS=y
>>>>> +CONFIG_DWC_ETH_QOS_QCOM=y
>>>>> +CONFIG_PHY=y
>>>>> +CONFIG_PINCTRL=y
>>>>> +CONFIG_PINCONF=y
>>>>> +CONFIG_PINCTRL_QCOM_SM8150=y
>>>>> +CONFIG_POWER_DOMAIN=y
>>>>> +CONFIG_MSM_GENI_SERIAL=y
>>>>> +CONFIG_SPMI_MSM=y
>>>>> +CONFIG_LMB_MAX_REGIONS=64
>>>>
>>>> Apart from above, I think this platform should be able to reuse
>>>> qcom_defconfig as you can find most of the config options there. Can
>>>> you try to reuse it?
>>>
>>> Honestly, the whole reason I am porting U-Boot to this platform is
>>> because I want to run Xen on it. And to run Xen, I need to run U-Boot in
>>> EL2. And to do this I need u-boot.elf with "correct" load address and
>>> entry point.
>>>
>>> I am planning to publish and upstream Xen patches as well (once I finish
>>> them). And it will be really nice if Xen users will be able use
>>> mainline U-Boot to boot Xen.
>>
>> I would like to enable the SM8150 drivers in qcom_defconfig (for
>> chainloading and supporting other platforms). But I'm totally fine with
>> having a separate defconfig for this board with this configuration.
> 
> Yes, this is a good approach. I'll do this.
> 
> [...]
>
Volodymyr Babchuk March 11, 2024, 6:23 p.m. UTC | #8
Hi Caleb,

Caleb Connolly <caleb.connolly@linaro.org> writes:

> On 06/03/2024 21:24, Volodymyr Babchuk wrote:
>> 
>> Hi Caleb,
>> 
>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>> 
>> [...]
>>>>>> +};
>>>>>> +
>>>>>> +&tlmm {
>>>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>>>> +       /delete-property/ reg-names;
>>>>>
>>>>> This won't be needed if we can make the tiles offset in the pinctrl
>>>>> driver compatible:
>>>>>
>>>>> #define WEST   0x00000000
>>>>> #define EAST   0x00400000
>>>>> #define NORTH  0x00800000
>>>>> #define SOUTH  0x00C00000
>>>>
>>>> Hmm, I assume that in this case pinctrl driver should map all the four
>>>> tiles independently? Are there guarantees in U-Boot that four separate
>>>> memory regions will be mapped into virtual memory with the same relative
>>>> positions? Linux clearly don't make such guarantees.
>>>
>>> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
>>> far as reading the address from DT, nothing else, so this is totally
>>> fine and is how the other SoCs do it.
>> 
>> For me it looks like we are depending on implementation details
>> knowledge. I.e MMU API does not provide such guarantees, but drivers
>> know how ARM MMU code is working internally and drivers depend on
>> exactly this behavior. But if you are saying that it is totally fine,
>> I'll rework the patch. No big deal. Actually, I already tried this and
>> it is working fine.
>> 
>>>>>> +
>>>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>>>> +       /delete-node/ phy-reset-pins;
>>>>>
>>>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>>>> as GPIO only.
>>>>>
>>>> Well, yes. This also puzzles me up, but for some reason it stops working
>>>> if I leave this node intact. Looks like I need to look at this deeper
>>>> before posting the next version.
>>>
>>> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
>>> misbehave, can you check if this patch fixes it (there is a bug in the
>>> line "return msm_gpio_direction_input(dev, gpio);", it should become
>>> just "msm_gpio_direction_input(dev, gpio);").
>>>
>>> I had the exact same issue with the gpio-regulator driver and this was
>>> the solution I ended up going with.
>>>
>>> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
>>> [lore[.]kernel[.]org]
>> 
>> It is exactly this. With your patch I don't need to /delete-node/
>> anymore. I'll add a comment in the cover message that this series are
>> depended on your patch.
>
> Please can you split the power domain and clock patches into a separate
> series? As I'd like to depend on them for the next revision of my
> series, and we'd otherwise have a cyclical dependency.

Of course.

As I understood, you are interested in "clk: qcom: clear div mask before
assigning a new divider" and "clk: qcom: add support for power domains
uclass", correct?
Caleb Connolly March 11, 2024, 6:51 p.m. UTC | #9
On 11/03/2024 18:23, Volodymyr Babchuk wrote:
> 
> Hi Caleb,
> 
> Caleb Connolly <caleb.connolly@linaro.org> writes:
> 
>> On 06/03/2024 21:24, Volodymyr Babchuk wrote:
>>>
>>> Hi Caleb,
>>>
>>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>>
>>> [...]
>>>>>>> +};
>>>>>>> +
>>>>>>> +&tlmm {
>>>>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>>>>> +       /delete-property/ reg-names;
>>>>>>
>>>>>> This won't be needed if we can make the tiles offset in the pinctrl
>>>>>> driver compatible:
>>>>>>
>>>>>> #define WEST   0x00000000
>>>>>> #define EAST   0x00400000
>>>>>> #define NORTH  0x00800000
>>>>>> #define SOUTH  0x00C00000
>>>>>
>>>>> Hmm, I assume that in this case pinctrl driver should map all the four
>>>>> tiles independently? Are there guarantees in U-Boot that four separate
>>>>> memory regions will be mapped into virtual memory with the same relative
>>>>> positions? Linux clearly don't make such guarantees.
>>>>
>>>> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
>>>> far as reading the address from DT, nothing else, so this is totally
>>>> fine and is how the other SoCs do it.
>>>
>>> For me it looks like we are depending on implementation details
>>> knowledge. I.e MMU API does not provide such guarantees, but drivers
>>> know how ARM MMU code is working internally and drivers depend on
>>> exactly this behavior. But if you are saying that it is totally fine,
>>> I'll rework the patch. No big deal. Actually, I already tried this and
>>> it is working fine.
>>>
>>>>>>> +
>>>>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>>>>> +       /delete-node/ phy-reset-pins;
>>>>>>
>>>>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>>>>> as GPIO only.
>>>>>>
>>>>> Well, yes. This also puzzles me up, but for some reason it stops working
>>>>> if I leave this node intact. Looks like I need to look at this deeper
>>>>> before posting the next version.
>>>>
>>>> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
>>>> misbehave, can you check if this patch fixes it (there is a bug in the
>>>> line "return msm_gpio_direction_input(dev, gpio);", it should become
>>>> just "msm_gpio_direction_input(dev, gpio);").
>>>>
>>>> I had the exact same issue with the gpio-regulator driver and this was
>>>> the solution I ended up going with.
>>>>
>>>> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
>>>> [lore[.]kernel[.]org]
>>>
>>> It is exactly this. With your patch I don't need to /delete-node/
>>> anymore. I'll add a comment in the cover message that this series are
>>> depended on your patch.
>>
>> Please can you split the power domain and clock patches into a separate
>> series? As I'd like to depend on them for the next revision of my
>> series, and we'd otherwise have a cyclical dependency.
> 
> Of course.
> 
> As I understood, you are interested in "clk: qcom: clear div mask before
> assigning a new divider" and "clk: qcom: add support for power domains
> uclass", correct?

Yes. I tried the power domain stuff out on SMD845 today and ran into 
quite a few issues. Specifically as a lot of the devices reference the 
rpmhpd power domain which we don't support (and don't *need* to support) 
in U-Boot. I'm not sure what the best way forward will be for this. 
Maybe a "nop" power domain driver?

Do you have the same issues on sm8150?
> 
>
Volodymyr Babchuk March 11, 2024, 8:11 p.m. UTC | #10
Caleb Connolly <caleb.connolly@linaro.org> writes:

> On 11/03/2024 18:23, Volodymyr Babchuk wrote:
>> Hi Caleb,
>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>> 
>>> On 06/03/2024 21:24, Volodymyr Babchuk wrote:
>>>>
>>>> Hi Caleb,
>>>>
>>>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>>>
>>>> [...]
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&tlmm {
>>>>>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>>>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>>>>>> +       /delete-property/ reg-names;
>>>>>>>
>>>>>>> This won't be needed if we can make the tiles offset in the pinctrl
>>>>>>> driver compatible:
>>>>>>>
>>>>>>> #define WEST   0x00000000
>>>>>>> #define EAST   0x00400000
>>>>>>> #define NORTH  0x00800000
>>>>>>> #define SOUTH  0x00C00000
>>>>>>
>>>>>> Hmm, I assume that in this case pinctrl driver should map all the four
>>>>>> tiles independently? Are there guarantees in U-Boot that four separate
>>>>>> memory regions will be mapped into virtual memory with the same relative
>>>>>> positions? Linux clearly don't make such guarantees.
>>>>>
>>>>> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
>>>>> far as reading the address from DT, nothing else, so this is totally
>>>>> fine and is how the other SoCs do it.
>>>>
>>>> For me it looks like we are depending on implementation details
>>>> knowledge. I.e MMU API does not provide such guarantees, but drivers
>>>> know how ARM MMU code is working internally and drivers depend on
>>>> exactly this behavior. But if you are saying that it is totally fine,
>>>> I'll rework the patch. No big deal. Actually, I already tried this and
>>>> it is working fine.
>>>>
>>>>>>>> +
>>>>>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>>>>>> +       /delete-node/ phy-reset-pins;
>>>>>>>
>>>>>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>>>>>> as GPIO only.
>>>>>>>
>>>>>> Well, yes. This also puzzles me up, but for some reason it stops working
>>>>>> if I leave this node intact. Looks like I need to look at this deeper
>>>>>> before posting the next version.
>>>>>
>>>>> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
>>>>> misbehave, can you check if this patch fixes it (there is a bug in the
>>>>> line "return msm_gpio_direction_input(dev, gpio);", it should become
>>>>> just "msm_gpio_direction_input(dev, gpio);").
>>>>>
>>>>> I had the exact same issue with the gpio-regulator driver and this was
>>>>> the solution I ended up going with.
>>>>>
>>>>> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
>>>>> [lore[.]kernel[.]org]
>>>>
>>>> It is exactly this. With your patch I don't need to /delete-node/
>>>> anymore. I'll add a comment in the cover message that this series are
>>>> depended on your patch.
>>>
>>> Please can you split the power domain and clock patches into a separate
>>> series? As I'd like to depend on them for the next revision of my
>>> series, and we'd otherwise have a cyclical dependency.
>> Of course.
>> As I understood, you are interested in "clk: qcom: clear div mask
>> before
>> assigning a new divider" and "clk: qcom: add support for power domains
>> uclass", correct?
>
> Yes.

Okay, I'll send it today.

> I tried the power domain stuff out on SMD845 today and ran into
> quite a few issues. Specifically as a lot of the devices reference the
> rpmhpd power domain which we don't support (and don't *need* to
> support) in U-Boot. I'm not sure what the best way forward will be for
> this. Maybe a "nop" power domain driver?

Are you sure that they are not required?

"nop" power domain always is the option. Especially if it prints some
warning about an unknown device. I had quite a lot of issues with clock and
pin drivers that silently ignore unknown devices...

> Do you have the same issues on sm8150?

Yes and no. No, because I was lucky so far and devices I tried to use in
U-Boot does not require rpmhpd. Looking at DTS, I may only encounter
issues with sdhc_2, which requires rpmhpd for some reason. Also UFS
requires clock from rpmhcc.

And "yes", because I have found root cause for my troubles with UFS in
Linux kernel, when I am skipping hyp.mbn. This is not strictly related
to U-Boot, but you may be interested in this: apparently Qualcomm's
hypervisor enables access to RPM (maybe brings it out of reset?). cmd-db
shared memory region can't be accessed if I skip the hypervisor and try
to boot directly into Linux. So now I am looking for ways to enable it.
Volodymyr Babchuk March 19, 2024, 9:18 p.m. UTC | #11
Hi,

Volodymyr Babchuk <volodymyr_babchuk@epam.com> writes:

> Caleb Connolly <caleb.connolly@linaro.org> writes:
>
>> On 11/03/2024 18:23, Volodymyr Babchuk wrote:
>>> Hi Caleb,
>>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>> 
>>>> On 06/03/2024 21:24, Volodymyr Babchuk wrote:
>>>>>
>>>>> Hi Caleb,
>>>>>
>>>>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>>>>
>>>>> [...]
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> +&tlmm {
>>>>>>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>>>>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>>>>>>> +       /delete-property/ reg-names;
>>>>>>>>
>>>>>>>> This won't be needed if we can make the tiles offset in the pinctrl
>>>>>>>> driver compatible:
>>>>>>>>
>>>>>>>> #define WEST   0x00000000
>>>>>>>> #define EAST   0x00400000
>>>>>>>> #define NORTH  0x00800000
>>>>>>>> #define SOUTH  0x00C00000
>>>>>>>
>>>>>>> Hmm, I assume that in this case pinctrl driver should map all the four
>>>>>>> tiles independently? Are there guarantees in U-Boot that four separate
>>>>>>> memory regions will be mapped into virtual memory with the same relative
>>>>>>> positions? Linux clearly don't make such guarantees.
>>>>>>
>>>>>> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
>>>>>> far as reading the address from DT, nothing else, so this is totally
>>>>>> fine and is how the other SoCs do it.
>>>>>
>>>>> For me it looks like we are depending on implementation details
>>>>> knowledge. I.e MMU API does not provide such guarantees, but drivers
>>>>> know how ARM MMU code is working internally and drivers depend on
>>>>> exactly this behavior. But if you are saying that it is totally fine,
>>>>> I'll rework the patch. No big deal. Actually, I already tried this and
>>>>> it is working fine.
>>>>>
>>>>>>>>> +
>>>>>>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>>>>>>> +       /delete-node/ phy-reset-pins;
>>>>>>>>
>>>>>>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>>>>>>> as GPIO only.
>>>>>>>>
>>>>>>> Well, yes. This also puzzles me up, but for some reason it stops working
>>>>>>> if I leave this node intact. Looks like I need to look at this deeper
>>>>>>> before posting the next version.
>>>>>>
>>>>>> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
>>>>>> misbehave, can you check if this patch fixes it (there is a bug in the
>>>>>> line "return msm_gpio_direction_input(dev, gpio);", it should become
>>>>>> just "msm_gpio_direction_input(dev, gpio);").
>>>>>>
>>>>>> I had the exact same issue with the gpio-regulator driver and this was
>>>>>> the solution I ended up going with.
>>>>>>
>>>>>> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
>>>>>> [lore[.]kernel[.]org]
>>>>>
>>>>> It is exactly this. With your patch I don't need to /delete-node/
>>>>> anymore. I'll add a comment in the cover message that this series are
>>>>> depended on your patch.
>>>>
>>>> Please can you split the power domain and clock patches into a separate
>>>> series? As I'd like to depend on them for the next revision of my
>>>> series, and we'd otherwise have a cyclical dependency.
>>> Of course.
>>> As I understood, you are interested in "clk: qcom: clear div mask
>>> before
>>> assigning a new divider" and "clk: qcom: add support for power domains
>>> uclass", correct?
>>
>> Yes.
>
> Okay, I'll send it today.
>
>> I tried the power domain stuff out on SMD845 today and ran into
>> quite a few issues. Specifically as a lot of the devices reference the
>> rpmhpd power domain which we don't support (and don't *need* to
>> support) in U-Boot. I'm not sure what the best way forward will be for
>> this. Maybe a "nop" power domain driver?
>
> Are you sure that they are not required?
>
> "nop" power domain always is the option. Especially if it prints some
> warning about an unknown device. I had quite a lot of issues with clock and
> pin drivers that silently ignore unknown devices...
>
>> Do you have the same issues on sm8150?
>
> Yes and no. No, because I was lucky so far and devices I tried to use in
> U-Boot does not require rpmhpd. Looking at DTS, I may only encounter
> issues with sdhc_2, which requires rpmhpd for some reason. Also UFS
> requires clock from rpmhcc.
>
> And "yes", because I have found root cause for my troubles with UFS in
> Linux kernel, when I am skipping hyp.mbn. This is not strictly related
> to U-Boot, but you may be interested in this: apparently Qualcomm's
> hypervisor enables access to RPM (maybe brings it out of reset?). cmd-db
> shared memory region can't be accessed if I skip the hypervisor and try
> to boot directly into Linux. So now I am looking for ways to enable it.

I want to share the solution, in case if someone got the same
problem. It all boiled down to correct memory attributes. Qualcomm's
hypervisor mapped cmd-db shared memory as non-cached in Stage 2
translation table, while Linux maps it as cacheable in Stage 1. Thus,
the final memory attribute was "non-cacheable".

Xen, on other hand, used default mapping which is normal cacheable
memory. And of course this lead to cacheable accesses to cmd-db. For
some reason this caused the hardware error, which manifested as a secure
interrupt (while I expected SError at most), which in turn led to
endless loop somewhere in TZ.

I am going to fix this by applying the correct mappings in the Linux
cmd-db driver. Plan B is to have workaround in Xen, but I really want to
avoid this.

Right now I have a working Xen that is able to boot Dom0 straight to
console, if anyone interested.
Caleb Connolly March 20, 2024, 1:18 p.m. UTC | #12
On 19/03/2024 21:18, Volodymyr Babchuk wrote:
> 
> Hi,
> 
> Volodymyr Babchuk <volodymyr_babchuk@epam.com> writes:
> 
>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>
>>> On 11/03/2024 18:23, Volodymyr Babchuk wrote:
>>>> Hi Caleb,
>>>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>>>
>>>>> On 06/03/2024 21:24, Volodymyr Babchuk wrote:
>>>>>>
>>>>>> Hi Caleb,
>>>>>>
>>>>>> Caleb Connolly <caleb.connolly@linaro.org> writes:
>>>>>>
>>>>>> [...]
>>>>>>>>>> +};
>>>>>>>>>> +
>>>>>>>>>> +&tlmm {
>>>>>>>>>> +       /* U-Boot pinctrl driver does not understand multiple tiles */
>>>>>>>>>> +       reg = <0x0 0x03000000 0x0 0x1000000>;
>>>>>>>>>> +       /delete-property/ reg-names;
>>>>>>>>>
>>>>>>>>> This won't be needed if we can make the tiles offset in the pinctrl
>>>>>>>>> driver compatible:
>>>>>>>>>
>>>>>>>>> #define WEST   0x00000000
>>>>>>>>> #define EAST   0x00400000
>>>>>>>>> #define NORTH  0x00800000
>>>>>>>>> #define SOUTH  0x00C00000
>>>>>>>>
>>>>>>>> Hmm, I assume that in this case pinctrl driver should map all the four
>>>>>>>> tiles independently? Are there guarantees in U-Boot that four separate
>>>>>>>> memory regions will be mapped into virtual memory with the same relative
>>>>>>>> positions? Linux clearly don't make such guarantees.
>>>>>>>
>>>>>>> U-Boot doesn't use virtual addresses on arm platforms, it only goes as
>>>>>>> far as reading the address from DT, nothing else, so this is totally
>>>>>>> fine and is how the other SoCs do it.
>>>>>>
>>>>>> For me it looks like we are depending on implementation details
>>>>>> knowledge. I.e MMU API does not provide such guarantees, but drivers
>>>>>> know how ARM MMU code is working internally and drivers depend on
>>>>>> exactly this behavior. But if you are saying that it is totally fine,
>>>>>> I'll rework the patch. No big deal. Actually, I already tried this and
>>>>>> it is working fine.
>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +       /* U-Boot ethernet driver wants to drive reset as GPIO */
>>>>>>>>>> +       /delete-node/ phy-reset-pins;
>>>>>>>>>
>>>>>>>>> I suppose this is not needed as phy-reset-pins also configures the pin
>>>>>>>>> as GPIO only.
>>>>>>>>>
>>>>>>>> Well, yes. This also puzzles me up, but for some reason it stops working
>>>>>>>> if I leave this node intact. Looks like I need to look at this deeper
>>>>>>>> before posting the next version.
>>>>>>>
>>>>>>> Possibly the pinconf defined in the phy-reset-pins node causes U-Boot to
>>>>>>> misbehave, can you check if this patch fixes it (there is a bug in the
>>>>>>> line "return msm_gpio_direction_input(dev, gpio);", it should become
>>>>>>> just "msm_gpio_direction_input(dev, gpio);").
>>>>>>>
>>>>>>> I had the exact same issue with the gpio-regulator driver and this was
>>>>>>> the solution I ended up going with.
>>>>>>>
>>>>>>> https://urldefense.com/v3/__https://lore.kernel.org/u-boot/20240131-b4-qcom-livetree-v1-7-4071c0787db0@linaro.org/__;!!GF_29dbcQIUBPA!xFhZe7DKgRbr63sirEJLuH-B0AnGs7jvx8tdJPKLTgFuZ3I3_zpVml7l23G-_vJO_JiUR-wUO4GMPJFcE-8p50H3pf7nbxit$
>>>>>>> [lore[.]kernel[.]org]
>>>>>>
>>>>>> It is exactly this. With your patch I don't need to /delete-node/
>>>>>> anymore. I'll add a comment in the cover message that this series are
>>>>>> depended on your patch.
>>>>>
>>>>> Please can you split the power domain and clock patches into a separate
>>>>> series? As I'd like to depend on them for the next revision of my
>>>>> series, and we'd otherwise have a cyclical dependency.
>>>> Of course.
>>>> As I understood, you are interested in "clk: qcom: clear div mask
>>>> before
>>>> assigning a new divider" and "clk: qcom: add support for power domains
>>>> uclass", correct?
>>>
>>> Yes.
>>
>> Okay, I'll send it today.
>>
>>> I tried the power domain stuff out on SMD845 today and ran into
>>> quite a few issues. Specifically as a lot of the devices reference the
>>> rpmhpd power domain which we don't support (and don't *need* to
>>> support) in U-Boot. I'm not sure what the best way forward will be for
>>> this. Maybe a "nop" power domain driver?
>>
>> Are you sure that they are not required?
>>
>> "nop" power domain always is the option. Especially if it prints some
>> warning about an unknown device. I had quite a lot of issues with clock and
>> pin drivers that silently ignore unknown devices...
>>
>>> Do you have the same issues on sm8150?
>>
>> Yes and no. No, because I was lucky so far and devices I tried to use in
>> U-Boot does not require rpmhpd. Looking at DTS, I may only encounter
>> issues with sdhc_2, which requires rpmhpd for some reason. Also UFS
>> requires clock from rpmhcc.
>>
>> And "yes", because I have found root cause for my troubles with UFS in
>> Linux kernel, when I am skipping hyp.mbn. This is not strictly related
>> to U-Boot, but you may be interested in this: apparently Qualcomm's
>> hypervisor enables access to RPM (maybe brings it out of reset?). cmd-db
>> shared memory region can't be accessed if I skip the hypervisor and try
>> to boot directly into Linux. So now I am looking for ways to enable it.
> 
> I want to share the solution, in case if someone got the same
> problem. It all boiled down to correct memory attributes. Qualcomm's
> hypervisor mapped cmd-db shared memory as non-cached in Stage 2
> translation table, while Linux maps it as cacheable in Stage 1. Thus,
> the final memory attribute was "non-cacheable".
> 
> Xen, on other hand, used default mapping which is normal cacheable
> memory. And of course this lead to cacheable accesses to cmd-db. For
> some reason this caused the hardware error, which manifested as a secure
> interrupt (while I expected SError at most), which in turn led to
> endless loop somewhere in TZ.

ohh, nice find! I guess if Linux started sending bogus data to the rpmh
this could definitely lead to some XPU violation or another excuse for a
secure interrupt...
> 
> I am going to fix this by applying the correct mappings in the Linux
> cmd-db driver. Plan B is to have workaround in Xen, but I really want to
> avoid this.

I agree this is definitely a Linux bug, it should make sense to fix it
there.
> 
> Right now I have a working Xen that is able to boot Dom0 straight to
> console, if anyone interested.

Awesome! Yeah I'd love to take a look :D
>
diff mbox series

Patch

diff --git a/arch/arm/dts/sa8155p-adp-u-boot.dtsi b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
new file mode 100644
index 0000000000..ffbf0933c7
--- /dev/null
+++ b/arch/arm/dts/sa8155p-adp-u-boot.dtsi
@@ -0,0 +1,30 @@ 
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Qualcomm SA8155P-ADP device tree fixups for U-BOot
+ *
+ * Volodymyr Babchuk <volodymyr_babchuk@epam.com>
+ * Copyright (c) 2024 EPAM Systems.
+ */
+
+/ {
+	/* Populate memory node with actual memory configuration */
+	memory@80000000 {
+		reg = <0x00 0x80000000 0x00 0x39900000>,
+		<0x02 0x0        0x1  0x7fd00000>,
+		<0x00 0xC0000000 0x1  0x40000000>;
+	};
+};
+
+&ethernet {
+	/* Ethernet driver tries to find reset by name */
+	reset-names = "emac";
+};
+
+&tlmm {
+	/* U-Boot pinctrl driver does not understand multiple tiles */
+	reg = <0x0 0x03000000 0x0 0x1000000>;
+	/delete-property/ reg-names;
+
+	/* U-Boot ethernet driver wants to drive reset as GPIO */
+	/delete-node/ phy-reset-pins;
+};
diff --git a/board/qualcomm/sa8155p-adp/MAINTAINERS b/board/qualcomm/sa8155p-adp/MAINTAINERS
new file mode 100644
index 0000000000..03fac84f51
--- /dev/null
+++ b/board/qualcomm/sa8155p-adp/MAINTAINERS
@@ -0,0 +1,5 @@ 
+Qualcomm SA8155P Automotive Development Platform
+M:	Volodymyr Babchuk <volodymyr_babchuk@epam.com>
+S:	Maintained
+F:	board/qualcomm/sa8155p-adp/
+F:	configs/sa8155p-adp_defconfig
diff --git a/configs/sa8155p_adp_defconfig b/configs/sa8155p_adp_defconfig
new file mode 100644
index 0000000000..b6969767f8
--- /dev/null
+++ b/configs/sa8155p_adp_defconfig
@@ -0,0 +1,35 @@ 
+CONFIG_ARM=y
+CONFIG_SKIP_LOWLEVEL_INIT=y
+CONFIG_COUNTER_FREQUENCY=19000000
+CONFIG_POSITION_INDEPENDENT=y
+CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
+CONFIG_ARCH_SNAPDRAGON=y
+CONFIG_TEXT_BASE=0x85710000
+CONFIG_DEFAULT_DEVICE_TREE="qcom/sa8155p-adp"
+CONFIG_IDENT_STRING="\nQualcomm SA8155P-ADP"
+CONFIG_SYS_LOAD_ADDR=0x85710000
+CONFIG_REMAKE_ELF=y
+CONFIG_BOOTDELAY=3
+CONFIG_SYS_CBSIZE=512
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_HUSH_PARSER=y
+CONFIG_OF_UPSTREAM=y
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_CLK=y
+CONFIG_CLK_QCOM_SM8150=y
+CONFIG_MSM_GPIO=y
+CONFIG_PHY_MICREL=y
+CONFIG_PHY_MICREL_KSZ90X1=y
+CONFIG_DM_MDIO=y
+CONFIG_DM_ETH_PHY=y
+CONFIG_DWC_ETH_QOS=y
+CONFIG_DWC_ETH_QOS_QCOM=y
+CONFIG_PHY=y
+CONFIG_PINCTRL=y
+CONFIG_PINCONF=y
+CONFIG_PINCTRL_QCOM_SM8150=y
+CONFIG_POWER_DOMAIN=y
+CONFIG_MSM_GENI_SERIAL=y
+CONFIG_SPMI_MSM=y
+CONFIG_LMB_MAX_REGIONS=64
diff --git a/doc/board/qualcomm/index.rst b/doc/board/qualcomm/index.rst
index 4955274a39..268218b05f 100644
--- a/doc/board/qualcomm/index.rst
+++ b/doc/board/qualcomm/index.rst
@@ -7,5 +7,6 @@  Qualcomm
    :maxdepth: 2
 
    dragonboard410c
+   sa8155p-adp
    board
    debugging
diff --git a/doc/board/qualcomm/sa8155p-adp.rst b/doc/board/qualcomm/sa8155p-adp.rst
new file mode 100644
index 0000000000..66db512b52
--- /dev/null
+++ b/doc/board/qualcomm/sa8155p-adp.rst
@@ -0,0 +1,87 @@ 
+.. SPDX-License-Identifier: BSD-3-Clause
+.. sectionauthor:: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
+
+SA8155P Automotive Development Platform
+=======================================
+
+About
+-----
+This document describes the information about SA8155P Automotive
+Development Platform aka SA8155P-ADP.
+
+Currently U-Boot can be booted either as Android boot image, or in EL2
+mode, instead of hypervisor image. In the latter case it is possible
+to use U-Boot to either boot Linux with KVM support or to boot Xen
+Hypervisor on this board.
+
+Supported HW modules
+^^^^^^^^^^^^^^^^^^^^
+Port for this board is in early development state. Right now U-Boot
+supports serial console and networking. No USB/fastboot or UFS support
+yet. So it is not possible to save environment variables as
+well. Nevertheless this is enough for development as user can download
+all required images via TFTP.
+
+Installation
+------------
+Build
+^^^^^
+Setup ``CROSS_COMPILE`` for aarch64 and build U-Boot for your board::
+
+	$ export CROSS_COMPILE=<aarch64 toolchain prefix>
+	$ make sa8155p_adp_defconfig
+	$ make
+
+This will build ``u-boot.bin`` in the configured output directory.
+
+Boot in EL1 mode instead of Android boot image
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Create a dummy ramdisk image:::
+
+	$ echo "This is not a ramdisk" > ramdisk.img
+
+Compress u-boot binary:::
+
+	$ gzip -c u-boot.bin > u-boot.bin.gz
+
+Append DTB again (binary we use already have DTB embedded in, but
+Android boot image format requires another DTB at the end of the
+archive):::
+
+	$ cat u-boot.bin.gz u-boot.dtb > u-boot.bin.gz-dtb
+
+Now we've got everything to build android boot image:::
+
+	$ mkbootimg --kernel u-boot.bin.gz-dtb \
+	--ramdisk ramdisk.img --pagesize 4096 \
+	--base 0x80000000 -o boot.img
+
+Finally you can flash new boot image with fastboot:::
+
+	$ fastboot flash boot boot.img
+
+Or just boot U-Boot without flashing anything:::
+
+	$ fastboot boot boot.img
+
+Boot in EL2 mode instead of Qualcomm's hypervisor stub
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+This approach ensures that U-Boot is booted in EL2 and it is possible
+to run virtualization software (like Xen or KVM) on the board. You
+must understand that this approach breaks Qualcomm's boot chain. You
+will not be able to call all subsequent loaders, so you will not be
+able to use fastboot for example. Use this approach only if you want
+to experiment with virtualization on SA8155P-ADP.
+
+U-Boot ELF file needs to be signed with test keys. `qtestsign
+<https://github.com/msm8916-mainline/qtestsign>`_ tool can be used ::
+
+	$ ../qtestsign/qtestsign.py -v6 hyp u-boot.elf
+
+Resulting ``u-boot-test-signed.mbn`` then can be written to the
+board. Easiest way is to use ``edl`` tool: ::
+
+	$ ../edl/edl w hyp_a u-boot-test-signed.mbn --memory=ufs --lun=4
+
+Be sure to backup existing hyp_a loader before flashing U-Boot.