Hi Tien Fong, On 16/4/2026 11:52 am, Chee, Tien Fong wrote: > Hi Dinesh, > > > On 11/3/2026 1:01 pm, [email protected] wrote: >> From: Dinesh Maniyam <[email protected]> >> >> By using the handoff COMBOPHY selection data [BIT 16], we can determine >> whether the combophy used for SDMMC or NAND. The DFISEL configured for >> SDMMC if the handoff COMBOPHY sel [BIT 16] selected for SDMMC. >> This ensures that the appropriate PHY interface is enabled during >> early boot based on handoff configuration. >> >> Signed-off-by: Dinesh Maniyam <[email protected]> >> --- >> .../mach-socfpga/include/mach/handoff_soc64.h | 1 + >> .../include/mach/system_manager_soc64.h | 1 + >> drivers/clk/altera/clk-agilex5.c | 16 ++++++++++++++++ >> 3 files changed, 18 insertions(+) >> >> diff --git a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h >> b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h >> index b8f2f73e283..39e3340f38c 100644 >> --- a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h >> +++ b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h >> @@ -81,6 +81,7 @@ >> #define SOC64_HANDOFF_PERI_LEN 1 >> #define SOC64_HANDOFF_SDRAM (SOC64_HANDOFF_BASE + 0x634) >> #define SOC64_HANDOFF_SDRAM_LEN 5 >> +#define SOC64_HANDOFF_COMBOPHY_SEL_MASK BIT(16) >> #endif >> >> #if IS_ENABLED(CONFIG_TARGET_SOCFPGA_STRATIX10) >> diff --git a/arch/arm/mach-socfpga/include/mach/system_manager_soc64.h >> b/arch/arm/mach-socfpga/include/mach/system_manager_soc64.h >> index f768a3a55cb..f95ee6ce226 100644 >> --- a/arch/arm/mach-socfpga/include/mach/system_manager_soc64.h >> +++ b/arch/arm/mach-socfpga/include/mach/system_manager_soc64.h >> @@ -18,6 +18,7 @@ void populate_sysmgr_pinmux(void); >> #define SYSMGR_SOC64_MPU_STATUS 0x10 >> #define SYSMGR_SOC64_COMBOPHY_DFISEL 0xfc >> #define SYSMGR_SOC64_COMBOPHY_DFISEL_SDMMC 0x1 >> +#define SYSMGR_SOC64_COMBOPHY_DFISEL_NAND 0x0 >> #define SYSMGR_SOC64_NANDGRP_L3MASTER 0x34 >> #define SYSMGR_SOC64_USB0_L3MASTER 0x38 >> #define SYSMGR_SOC64_USB1_L3MASTER 0x3c >> diff --git a/drivers/clk/altera/clk-agilex5.c >> b/drivers/clk/altera/clk-agilex5.c >> index fb1e72ffc5c..dd999e25aec 100644 >> --- a/drivers/clk/altera/clk-agilex5.c >> +++ b/drivers/clk/altera/clk-agilex5.c >> @@ -25,6 +25,9 @@ >> #include <dt-bindings/clock/agilex5-clock.h> >> #include <wait_bit.h> >> #include <clk-uclass.h> >> +#include <asm/arch/system_manager.h> >> +#include <asm/arch/system_manager_soc64.h> >> +#include <asm/arch/firewall.h> > > > #include <asm/arch/firewall.h> does not appear to be used by this file > after the change. Please drop it unless a follow-up hunk needs it. > > >> >> DECLARE_GLOBAL_DATA_PTR; >> >> @@ -244,10 +247,15 @@ static void clk_basic_init(struct udevice *dev, >> struct socfpga_clk_plat *plat = dev_get_plat(dev); >> u32 vcocalib; >> uintptr_t base_addr = (uintptr_t)plat->regs; >> + u32 len = SOC64_HANDOFF_PERI_LEN; >> + u32 handoff_table[len]; > > > u32 handoff_table[SOC64_HANDOFF_PERI_LEN]; is preferred to avoid > variable-length array > >> >> if (!cfg) >> return; >> >> + /* Read handoff for PWRGATE configuration */ > > > The new socfpga_handoff_read() is described as “Read handoff for PWRGATE > configuration”, but handoff_table[] is only used for COMBOPHY / > SYSMGR_SOC64_COMBOPHY_DFISEL selection. Please update or remove that > comment so it reflects the actual purpose (PERI handoff word used for > COMBOPHY select bit). > > >> + socfpga_handoff_read((void *)SOC64_HANDOFF_PERI, handoff_table, len); > > > Update len with SOC64_HANDOFF_PERI_LEN > > [...] > > > Best regards, > > Tien Fong >
Thanks for the review. Agreed on all points: - drop the unused firewall.h include - use a fixed-size handoff_table[SOC64_HANDOFF_PERI_LEN] - fix the comment to describe COMBOPHY/DFISEL selection from the PERI handoff word - pass SOC64_HANDOFF_PERI_LEN directly to socfpga_handoff_read() I'll send v2 shortly. Best regards, Dinesh
