On Tue, Feb 27, 2024 at 02:37:25AM +0000, Caleb Connolly wrote:
HI Varada,
On 26/02/2024 10:07, Varadarajan Narayanan wrote:
These patches introduce the initial support code needed for the QTI IPQ9574 SoC and RDP433 board.
Awesome!
Thanks for the feedback. Will rework based on the comments given in this and the other patches and will post a new spin soon.
-Varada
SoC : QTI IPQ9574 RAM : 2GB DDR4 Flash : eMMC 8GB WiFi : 1 x 2.4GHz, 1 x 5GHz, 1 x 6GHz
New to both patman and posting to U-Boot. Please let me know if something is not correct, will try to rectify to the best of my abilities.
Patches came through just fine, thanks! I haven't used patman personally, but I have used the b4 tool, I'd recommend giving it a spin if you haven't yet. Your get_maintainers.pl options might need some tweaking too (the defaults from b4 seem to work better ime).
I've spent some time going through your series and there's a lot to go over, so apologies if this gets a bit long.
I'm planning to pick up this[1] patch series this week and I'm sorry to say it has some fairly major conflicts with your work here. My series makes some fairly significant changes to Qualcomm support in U-Boot (moving to upstream Linux DT and getting rid of most of the compile time board configuration among other things).
The thing I'm mostly concerned with is the totally custom init code and linker script. I discovered this branch[2] which seems to have a fairly complete git history, but there's still a lot of unknowns.
- What is the custom linker script for (which isn't handled by the
default one)?
- Why do you skip U-Boot relocation (GD_FLG_SKIP_RELOC)?
- How is U-Boot expected to be flashed and consequently loaded on this
platform? I get the impression that SBL1 jumps to it directly? Is it built into an XBL image like on the mobile/IoT platforms, or flashed to it's own partition, or to SPI flash?
In general there is very little context around the code being added here, and as a result it's hard to understand what exactly is going on and why things have been done a certain way.
I'll leave some more detailed comments in line, but I think initially I would just like to get a better understanding of the situation here, so answers to the questions above would be most helpful!
Please also have a look at reworking this series on top of my patches linked at [1], if you have any issues or questions then please do get in touch with me via email, or better via the #u-boot IRC channel on OFTC, my nick there is calebccff.
Regarding my series, the notable changes are in how mach-snapdragon is handled: the target specific memory maps have been dropped in favour of reading the memory map from DT. The sysmap headers are also gone, instead the defines are moved to the relevant driver (mostly clock/pinctrl).
I think it would be sensible to (for example) introduce mach-snapdragon/smem.c for handling stuff like your env_get_location() implementation (it would provide smem_env_get_location() which would be called from env_get_location() in board.c).
You'll probably also want to skip the carve_out_reserved_memory() call for now.
Thanks Varada
Kind regards,
Varadarajan Narayanan (10): mach-snapdragon: Add support for IPQ9574 mach-snapdragon: ipq9574: Add SMEM defines needed for IPQ9574 clk/qcom: Add support for clock registers in IPQ9574 clk/qcom: Include IPQ9574 mach-snapdragon: Update gd->ram_size in msm_fixup_memory pinctrl: qcom: Add support for 'pull-down' pinctrl: qcom: Include IPQ9574 mmc: msm_sdhci: Handle different vendor cap register offsets mmc: msm_sdhci: Reset clocks before reconfiguration board: qualcomm: Add support for IPQ9574 RDP433
arch/arm/dts/Makefile | 2 + arch/arm/dts/ipq9574-default.dts | 167 +++ arch/arm/dts/ipq9574-rdp433-mht-phy.dts | 208 +++ arch/arm/dts/ipq9574.dtsi | 771 ++++++++++ arch/arm/mach-snapdragon/Kconfig | 15 + arch/arm/mach-snapdragon/Makefile | 1 + arch/arm/mach-snapdragon/dram.c | 10 +- .../include/mach/sysmap-ipq9574.h | 252 ++++ arch/arm/mach-snapdragon/init_ipq9574.c | 81 + board/qualcomm/ipq9574/Kconfig | 15 + board/qualcomm/ipq9574/Makefile | 4 + board/qualcomm/ipq9574/board_init.c | 326 ++++ board/qualcomm/ipq9574/ipq9574.c | 170 +++ board/qualcomm/ipq9574/ipq9574.h | 75 + board/qualcomm/ipq9574/u-boot-x32.lds | 250 ++++ board/qualcomm/ipq9574/u-boot-x64.lds | 188 +++ configs/ipq9574_mmc_defconfig | 118 ++ drivers/clk/qcom/Kconfig | 8 + drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/clock-ipq9574.c | 1320 +++++++++++++++++ drivers/clk/qcom/clock-qcom.c | 32 + drivers/clk/qcom/clock-qcom.h | 8 + drivers/mmc/msm_sdhci.c | 21 +- drivers/pinctrl/qcom/Kconfig | 7 + drivers/pinctrl/qcom/Makefile | 1 + drivers/pinctrl/qcom/pinctrl-ipq9574.c | 77 + drivers/pinctrl/qcom/pinctrl-qcom.c | 2 + include/configs/ipq9574.h | 111 ++ include/dt-bindings/clock/gcc-ipq9574.h | 156 ++ include/dt-bindings/net/qti-ipqsoc.h | 20 + include/dt-bindings/pinctrl/pinctrl-ipqsoc.h | 19 + include/dt-bindings/reset/ipq9574-reset.h | 54 + include/smem.h | 78 + 33 files changed, 4563 insertions(+), 5 deletions(-) create mode 100644 arch/arm/dts/ipq9574-default.dts create mode 100644 arch/arm/dts/ipq9574-rdp433-mht-phy.dts create mode 100644 arch/arm/dts/ipq9574.dtsi create mode 100644 arch/arm/mach-snapdragon/include/mach/sysmap-ipq9574.h create mode 100644 arch/arm/mach-snapdragon/init_ipq9574.c create mode 100644 board/qualcomm/ipq9574/Kconfig create mode 100644 board/qualcomm/ipq9574/Makefile create mode 100644 board/qualcomm/ipq9574/board_init.c create mode 100644 board/qualcomm/ipq9574/ipq9574.c create mode 100644 board/qualcomm/ipq9574/ipq9574.h create mode 100644 board/qualcomm/ipq9574/u-boot-x32.lds create mode 100644 board/qualcomm/ipq9574/u-boot-x64.lds create mode 100644 configs/ipq9574_mmc_defconfig create mode 100644 drivers/clk/qcom/clock-ipq9574.c create mode 100644 drivers/pinctrl/qcom/pinctrl-ipq9574.c create mode 100644 include/configs/ipq9574.h create mode 100644 include/dt-bindings/clock/gcc-ipq9574.h create mode 100644 include/dt-bindings/net/qti-ipqsoc.h create mode 100644 include/dt-bindings/pinctrl/pinctrl-ipqsoc.h create mode 100644 include/dt-bindings/reset/ipq9574-reset.h
-- // Caleb (they/them)