| Message ID | 20250430104236.2611089-2-a-patra@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Add fdt-fixups for AM62A variants | expand |
On April 30, 2025 thus sayeth Aparna Patra: > AM62A SOC is available in multiple variants: > -CPU cores (Cortex-A) AM62Ax1 (1 core), > AM62Ax2 (2 cores), AM62Ax4 (4 cores) > -With and without DSS, CAN-FD & Video-codec support > > Remove the relevant FDT nodes by reading the actual configuration > from the SoC registers, with that change it is possible to have a single > dts/dtb file handling the different variants at runtime. > > Signed-off-by: Aparna Patra <a-patra@ti.com> > --- > arch/arm/mach-k3/am62ax/am62a7_fdt.c | 41 +++++++++++++++++++ > .../arm/mach-k3/include/mach/am62a_hardware.h | 40 ++++++++++++++++++ > 2 files changed, 81 insertions(+) > > diff --git a/arch/arm/mach-k3/am62ax/am62a7_fdt.c b/arch/arm/mach-k3/am62ax/am62a7_fdt.c > index 7f764ab36b..56fd658202 100644 > --- a/arch/arm/mach-k3/am62ax/am62a7_fdt.c > +++ b/arch/arm/mach-k3/am62ax/am62a7_fdt.c > @@ -8,8 +8,49 @@ > > #include "../common_fdt.h" > > +static void fdt_fixup_cores_wdt_nodes_am62a(void *blob, int core_nr) > +{ > + char node_path[32]; > + > + if (core_nr < 1) > + return; > + > + for (; core_nr < 4; core_nr++) { > + snprintf(node_path, sizeof(node_path), "/cpus/cpu@%d", core_nr); > + fdt_del_node_path(blob, node_path); > + snprintf(node_path, sizeof(node_path), "/cpus/cpu-map/cluster0/core%d", core_nr); > + fdt_del_node_path(blob, node_path); > + snprintf(node_path, sizeof(node_path), "/bus@f0000/watchdog@e0%d0000", core_nr); > + fdt_del_node_path(blob, node_path); > + } > +} > + > +static void fdt_fixup_dss_nodes_am62a(void *blob, bool has_dss) > +{ > + if (!has_dss) { > + fdt_del_node_path(blob, "/bus@f0000/dss@30200000"); > + fdt_del_node_path(blob, "/bus@f0000/pinctrl@f4000/main-dss0-default-pins"); So for a little background on the upcoming paragraph[0] I completely agree we should remove the dss{} node but don't think it's wise of us to start trying to unwind the allocation or configuration of the hardware when the hardware isn't there. In my eyes because we can't really trust the names for the memory carveouts, mailbox and, pinmux nodes as they are just describing their use and are fairly unstable as the boards age we would need to keep up with each node's bindings and follow all the pinmux, mboxes, memory-region phandles via their properties to undo all those nodes which leaves us making too many opinionated decisions on how best to do that in my opinion. Anywho from that first link I'm trusting Andrew has a plan which from what I've seen could look something like an overlay[1] But time will tell. ~Bryan [0] https://lore.kernel.org/all/20250419150451.v3jgtgp4yisou65u@bryanbrattlof.com/ [1] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?id=95010a7c186b86f738615231016aa017ac527612
On 16:12-20250430, Aparna Patra wrote: > AM62A SOC is available in multiple variants: > -CPU cores (Cortex-A) AM62Ax1 (1 core), > AM62Ax2 (2 cores), AM62Ax4 (4 cores) > -With and without DSS, CAN-FD & Video-codec support > > Remove the relevant FDT nodes by reading the actual configuration > from the SoC registers, with that change it is possible to have a single > dts/dtb file handling the different variants at runtime. > > Signed-off-by: Aparna Patra <a-patra@ti.com> > --- > arch/arm/mach-k3/am62ax/am62a7_fdt.c | 41 +++++++++++++++++++ > .../arm/mach-k3/include/mach/am62a_hardware.h | 40 ++++++++++++++++++ > 2 files changed, 81 insertions(+) > > diff --git a/arch/arm/mach-k3/am62ax/am62a7_fdt.c b/arch/arm/mach-k3/am62ax/am62a7_fdt.c > index 7f764ab36b..56fd658202 100644 > --- a/arch/arm/mach-k3/am62ax/am62a7_fdt.c > +++ b/arch/arm/mach-k3/am62ax/am62a7_fdt.c > @@ -8,8 +8,49 @@ > > #include "../common_fdt.h" > > +static void fdt_fixup_cores_wdt_nodes_am62a(void *blob, int core_nr) > +{ > + char node_path[32]; > + > + if (core_nr < 1) > + return; > + > + for (; core_nr < 4; core_nr++) { > + snprintf(node_path, sizeof(node_path), "/cpus/cpu@%d", core_nr); > + fdt_del_node_path(blob, node_path); > + snprintf(node_path, sizeof(node_path), "/cpus/cpu-map/cluster0/core%d", core_nr); > + fdt_del_node_path(blob, node_path); > + snprintf(node_path, sizeof(node_path), "/bus@f0000/watchdog@e0%d0000", core_nr); > + fdt_del_node_path(blob, node_path); > + } > +} > + > +static void fdt_fixup_dss_nodes_am62a(void *blob, bool has_dss) > +{ > + if (!has_dss) { > + fdt_del_node_path(blob, "/bus@f0000/dss@30200000"); > + fdt_del_node_path(blob, "/bus@f0000/pinctrl@f4000/main-dss0-default-pins"); > + } > +} > + > +static void fdt_fixup_video_codec_nodes_am62a(void *blob, bool has_video_codec) > +{ > + if (!has_video_codec) > + fdt_del_node_path(blob, "/bus@f0000/video-codec@30210000"); > +} > + > +static void fdt_fixup_canfd_nodes_am62a(void *blob, bool has_canfd) > +{ > + if (!has_canfd) > + fdt_del_node_path(blob, "/bus@f0000/can@20701000"); > +} > + > int ft_system_setup(void *blob, struct bd_info *bd) > { > + fdt_fixup_cores_wdt_nodes_am62a(blob, k3_get_core_nr()); > + fdt_fixup_dss_nodes_am62a(blob, k3_has_dss()); > + fdt_fixup_video_codec_nodes_am62a(blob, k3_has_video_codec()); > + fdt_fixup_canfd_nodes_am62a(blob, k3_has_canfd()); > fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000); > fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000); > > diff --git a/arch/arm/mach-k3/include/mach/am62a_hardware.h b/arch/arm/mach-k3/include/mach/am62a_hardware.h > index cd61abe018..ab281129d7 100644 > --- a/arch/arm/mach-k3/include/mach/am62a_hardware.h > +++ b/arch/arm/mach-k3/include/mach/am62a_hardware.h > @@ -19,6 +19,18 @@ > #define MCU_CTRL_MMR0_BASE 0x04500000 > #define WKUP_CTRL_MMR0_BASE 0x43000000 > > +#define CTRLMMR_WKUP_JTAG_DEVICE_ID (WKUP_CTRL_MMR0_BASE + 0x18) > +#define JTAG_DEV_CORE_NR_MASK GENMASK(19, 18) > +#define JTAG_DEV_CORE_NR_SHIFT 18 > +#define JTAG_DEV_CANFD_MASK BIT(15) > +#define JTAG_DEV_CANFD_SHIFT 15 > +#define JTAG_DEV_VIDEO_CODEC_MASK BIT(14) > +#define JTAG_DEV_VIDEO_CODEC_SHIFT 14 > +#define JTAG_DEV_DSS_MASK GENMASK(16, 13) > +#define JTAG_DEV_DSS_SHIFT 13 > + > +#define JTAG_DEV_HAS_DSS_VALUE 0xD > + > #define CTRLMMR_MAIN_DEVSTAT (WKUP_CTRL_MMR0_BASE + 0x30) > #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK GENMASK(6, 3) > #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT 3 > @@ -86,6 +98,34 @@ > #define TI_SRAM_SCRATCH_BOARD_EEPROM_START 0x70000001 > #endif /* CONFIG_CPU_V7R */ > > +static inline int k3_get_core_nr(void) > +{ > + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); > + > + return ((dev_id & JTAG_DEV_CORE_NR_MASK) >> JTAG_DEV_CORE_NR_SHIFT) + 1; > +} > + > +static inline int k3_has_dss(void) > +{ > + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); > + > + return (JTAG_DEV_HAS_DSS_VALUE == ((dev_id & JTAG_DEV_DSS_MASK) >> JTAG_DEV_DSS_SHIFT)); > +} > + > +static inline int k3_has_video_codec(void) > +{ > + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); > + > + return !((dev_id & JTAG_DEV_VIDEO_CODEC_MASK) >> JTAG_DEV_VIDEO_CODEC_SHIFT); > +} > + > +static inline int k3_has_canfd(void) > +{ > + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); > + > + return (dev_id & JTAG_DEV_CANFD_MASK) >> JTAG_DEV_CANFD_SHIFT; > +} > + > #if defined(CONFIG_SYS_K3_SPL_ATF) && !defined(__ASSEMBLY__) > > static const u32 put_device_ids[] = {}; > -- > 2.34.1 > We should do a proper framework for this that scales multiple devices.
On 5/1/25 14:28, Nishanth Menon wrote: > On 16:12-20250430, Aparna Patra wrote: >> AM62A SOC is available in multiple variants: >> -CPU cores (Cortex-A) AM62Ax1 (1 core), >> AM62Ax2 (2 cores), AM62Ax4 (4 cores) >> -With and without DSS, CAN-FD & Video-codec support >> >> Remove the relevant FDT nodes by reading the actual configuration >> from the SoC registers, with that change it is possible to have a single >> dts/dtb file handling the different variants at runtime. >> >> Signed-off-by: Aparna Patra <a-patra@ti.com> >> --- >> arch/arm/mach-k3/am62ax/am62a7_fdt.c | 41 +++++++++++++++++++ >> .../arm/mach-k3/include/mach/am62a_hardware.h | 40 ++++++++++++++++++ >> 2 files changed, 81 insertions(+) >> >> diff --git a/arch/arm/mach-k3/am62ax/am62a7_fdt.c b/arch/arm/mach-k3/am62ax/am62a7_fdt.c >> index 7f764ab36b..56fd658202 100644 >> --- a/arch/arm/mach-k3/am62ax/am62a7_fdt.c >> +++ b/arch/arm/mach-k3/am62ax/am62a7_fdt.c >> @@ -8,8 +8,49 @@ >> >> #include "../common_fdt.h" >> >> +static void fdt_fixup_cores_wdt_nodes_am62a(void *blob, int core_nr) >> +{ >> + char node_path[32]; >> + >> + if (core_nr < 1) >> + return; >> + >> + for (; core_nr < 4; core_nr++) { >> + snprintf(node_path, sizeof(node_path), "/cpus/cpu@%d", core_nr); >> + fdt_del_node_path(blob, node_path); >> + snprintf(node_path, sizeof(node_path), "/cpus/cpu-map/cluster0/core%d", core_nr); >> + fdt_del_node_path(blob, node_path); >> + snprintf(node_path, sizeof(node_path), "/bus@f0000/watchdog@e0%d0000", core_nr); >> + fdt_del_node_path(blob, node_path); >> + } >> +} >> + >> +static void fdt_fixup_dss_nodes_am62a(void *blob, bool has_dss) >> +{ >> + if (!has_dss) { >> + fdt_del_node_path(blob, "/bus@f0000/dss@30200000"); >> + fdt_del_node_path(blob, "/bus@f0000/pinctrl@f4000/main-dss0-default-pins"); >> + } >> +} >> + >> +static void fdt_fixup_video_codec_nodes_am62a(void *blob, bool has_video_codec) >> +{ >> + if (!has_video_codec) >> + fdt_del_node_path(blob, "/bus@f0000/video-codec@30210000"); >> +} >> + >> +static void fdt_fixup_canfd_nodes_am62a(void *blob, bool has_canfd) >> +{ >> + if (!has_canfd) >> + fdt_del_node_path(blob, "/bus@f0000/can@20701000"); >> +} >> + >> int ft_system_setup(void *blob, struct bd_info *bd) >> { >> + fdt_fixup_cores_wdt_nodes_am62a(blob, k3_get_core_nr()); >> + fdt_fixup_dss_nodes_am62a(blob, k3_has_dss()); >> + fdt_fixup_video_codec_nodes_am62a(blob, k3_has_video_codec()); >> + fdt_fixup_canfd_nodes_am62a(blob, k3_has_canfd()); >> fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000); >> fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000); >> >> diff --git a/arch/arm/mach-k3/include/mach/am62a_hardware.h b/arch/arm/mach-k3/include/mach/am62a_hardware.h >> index cd61abe018..ab281129d7 100644 >> --- a/arch/arm/mach-k3/include/mach/am62a_hardware.h >> +++ b/arch/arm/mach-k3/include/mach/am62a_hardware.h >> @@ -19,6 +19,18 @@ >> #define MCU_CTRL_MMR0_BASE 0x04500000 >> #define WKUP_CTRL_MMR0_BASE 0x43000000 >> >> +#define CTRLMMR_WKUP_JTAG_DEVICE_ID (WKUP_CTRL_MMR0_BASE + 0x18) >> +#define JTAG_DEV_CORE_NR_MASK GENMASK(19, 18) >> +#define JTAG_DEV_CORE_NR_SHIFT 18 >> +#define JTAG_DEV_CANFD_MASK BIT(15) >> +#define JTAG_DEV_CANFD_SHIFT 15 >> +#define JTAG_DEV_VIDEO_CODEC_MASK BIT(14) >> +#define JTAG_DEV_VIDEO_CODEC_SHIFT 14 >> +#define JTAG_DEV_DSS_MASK GENMASK(16, 13) >> +#define JTAG_DEV_DSS_SHIFT 13 >> + >> +#define JTAG_DEV_HAS_DSS_VALUE 0xD >> + >> #define CTRLMMR_MAIN_DEVSTAT (WKUP_CTRL_MMR0_BASE + 0x30) >> #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK GENMASK(6, 3) >> #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT 3 >> @@ -86,6 +98,34 @@ >> #define TI_SRAM_SCRATCH_BOARD_EEPROM_START 0x70000001 >> #endif /* CONFIG_CPU_V7R */ >> >> +static inline int k3_get_core_nr(void) >> +{ >> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >> + >> + return ((dev_id & JTAG_DEV_CORE_NR_MASK) >> JTAG_DEV_CORE_NR_SHIFT) + 1; >> +} >> + I can see these three following components are optional but I don't see any silicon devices without them. The AM62A processor data sheet only lists devices with different A53 and C7 options. BTW: C7x fixup is missing in this series ;) >> +static inline int k3_has_dss(void) >> +{ >> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >> + >> + return (JTAG_DEV_HAS_DSS_VALUE == ((dev_id & JTAG_DEV_DSS_MASK) >> JTAG_DEV_DSS_SHIFT)); >> +} >> + >> +static inline int k3_has_video_codec(void) >> +{ >> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >> + >> + return !((dev_id & JTAG_DEV_VIDEO_CODEC_MASK) >> JTAG_DEV_VIDEO_CODEC_SHIFT); >> +} >> + >> +static inline int k3_has_canfd(void) >> +{ >> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >> + >> + return (dev_id & JTAG_DEV_CANFD_MASK) >> JTAG_DEV_CANFD_SHIFT; >> +} >> + >> #if defined(CONFIG_SYS_K3_SPL_ATF) && !defined(__ASSEMBLY__) >> >> static const u32 put_device_ids[] = {}; >> -- >> 2.34.1 >> > We should do a proper framework for this that scales multiple devices. I agree, this should be reworked afterwards and moved into a framework. We have literally the same logic for AM62 and AM62Ax. I also have temperature fixup patches for the AM64 which are identical again. Would be nice to move most of this logic into a common part. Thanks for sending those patches! - Daniel > >
On 02/05/25 15:08, Daniel Schultz wrote: > > On 5/1/25 14:28, Nishanth Menon wrote: >> On 16:12-20250430, Aparna Patra wrote: >>> AM62A SOC is available in multiple variants: >>> -CPU cores (Cortex-A) AM62Ax1 (1 core), >>> AM62Ax2 (2 cores), AM62Ax4 (4 cores) >>> -With and without DSS, CAN-FD & Video-codec support >>> >>> Remove the relevant FDT nodes by reading the actual configuration >>> from the SoC registers, with that change it is possible to have a >>> single >>> dts/dtb file handling the different variants at runtime. >>> >>> Signed-off-by: Aparna Patra <a-patra@ti.com> >>> --- >>> arch/arm/mach-k3/am62ax/am62a7_fdt.c | 41 >>> +++++++++++++++++++ >>> .../arm/mach-k3/include/mach/am62a_hardware.h | 40 ++++++++++++++++++ >>> 2 files changed, 81 insertions(+) >>> >>> diff --git a/arch/arm/mach-k3/am62ax/am62a7_fdt.c >>> b/arch/arm/mach-k3/am62ax/am62a7_fdt.c >>> index 7f764ab36b..56fd658202 100644 >>> --- a/arch/arm/mach-k3/am62ax/am62a7_fdt.c >>> +++ b/arch/arm/mach-k3/am62ax/am62a7_fdt.c >>> @@ -8,8 +8,49 @@ >>> #include "../common_fdt.h" >>> +static void fdt_fixup_cores_wdt_nodes_am62a(void *blob, int core_nr) >>> +{ >>> + char node_path[32]; >>> + >>> + if (core_nr < 1) >>> + return; >>> + >>> + for (; core_nr < 4; core_nr++) { >>> + snprintf(node_path, sizeof(node_path), "/cpus/cpu@%d", >>> core_nr); >>> + fdt_del_node_path(blob, node_path); >>> + snprintf(node_path, sizeof(node_path), >>> "/cpus/cpu-map/cluster0/core%d", core_nr); >>> + fdt_del_node_path(blob, node_path); >>> + snprintf(node_path, sizeof(node_path), >>> "/bus@f0000/watchdog@e0%d0000", core_nr); >>> + fdt_del_node_path(blob, node_path); >>> + } >>> +} >>> + >>> +static void fdt_fixup_dss_nodes_am62a(void *blob, bool has_dss) >>> +{ >>> + if (!has_dss) { >>> + fdt_del_node_path(blob, "/bus@f0000/dss@30200000"); >>> + fdt_del_node_path(blob, >>> "/bus@f0000/pinctrl@f4000/main-dss0-default-pins"); >>> + } >>> +} >>> + >>> +static void fdt_fixup_video_codec_nodes_am62a(void *blob, bool >>> has_video_codec) >>> +{ >>> + if (!has_video_codec) >>> + fdt_del_node_path(blob, "/bus@f0000/video-codec@30210000"); >>> +} >>> + >>> +static void fdt_fixup_canfd_nodes_am62a(void *blob, bool has_canfd) >>> +{ >>> + if (!has_canfd) >>> + fdt_del_node_path(blob, "/bus@f0000/can@20701000"); >>> +} >>> + >>> int ft_system_setup(void *blob, struct bd_info *bd) >>> { >>> + fdt_fixup_cores_wdt_nodes_am62a(blob, k3_get_core_nr()); >>> + fdt_fixup_dss_nodes_am62a(blob, k3_has_dss()); >>> + fdt_fixup_video_codec_nodes_am62a(blob, k3_has_video_codec()); >>> + fdt_fixup_canfd_nodes_am62a(blob, k3_has_canfd()); >>> fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, >>> 0x80000); >>> fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, >>> 0x1800000); >>> diff --git a/arch/arm/mach-k3/include/mach/am62a_hardware.h >>> b/arch/arm/mach-k3/include/mach/am62a_hardware.h >>> index cd61abe018..ab281129d7 100644 >>> --- a/arch/arm/mach-k3/include/mach/am62a_hardware.h >>> +++ b/arch/arm/mach-k3/include/mach/am62a_hardware.h >>> @@ -19,6 +19,18 @@ >>> #define MCU_CTRL_MMR0_BASE 0x04500000 >>> #define WKUP_CTRL_MMR0_BASE 0x43000000 >>> +#define CTRLMMR_WKUP_JTAG_DEVICE_ID (WKUP_CTRL_MMR0_BASE + 0x18) >>> +#define JTAG_DEV_CORE_NR_MASK GENMASK(19, 18) >>> +#define JTAG_DEV_CORE_NR_SHIFT 18 >>> +#define JTAG_DEV_CANFD_MASK BIT(15) >>> +#define JTAG_DEV_CANFD_SHIFT 15 >>> +#define JTAG_DEV_VIDEO_CODEC_MASK BIT(14) >>> +#define JTAG_DEV_VIDEO_CODEC_SHIFT 14 >>> +#define JTAG_DEV_DSS_MASK GENMASK(16, 13) >>> +#define JTAG_DEV_DSS_SHIFT 13 >>> + >>> +#define JTAG_DEV_HAS_DSS_VALUE 0xD >>> + >>> #define CTRLMMR_MAIN_DEVSTAT (WKUP_CTRL_MMR0_BASE + 0x30) >>> #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK GENMASK(6, 3) >>> #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT 3 >>> @@ -86,6 +98,34 @@ >>> #define TI_SRAM_SCRATCH_BOARD_EEPROM_START 0x70000001 >>> #endif /* CONFIG_CPU_V7R */ >>> +static inline int k3_get_core_nr(void) >>> +{ >>> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >>> + >>> + return ((dev_id & JTAG_DEV_CORE_NR_MASK) >> >>> JTAG_DEV_CORE_NR_SHIFT) + 1; >>> +} >>> + > > I can see these three following components are optional but I don't > see any silicon devices without them. The AM62A processor data sheet > only lists devices with different A53 and C7 options. > > BTW: C7x fixup is missing in this series ;) Daniel, table 4-1 only shows the classification of AM62A family. please refer to table 9.1 in datasheet for device grades information. There are device grades which doesnot have DSS, Video-codec etc For C7x, upstream does not include C7x device nodes as of now. Fixups will be added once nodes are included in dts. > >>> +static inline int k3_has_dss(void) >>> +{ >>> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >>> + >>> + return (JTAG_DEV_HAS_DSS_VALUE == ((dev_id & JTAG_DEV_DSS_MASK) >>> >> JTAG_DEV_DSS_SHIFT)); >>> +} >>> + >>> +static inline int k3_has_video_codec(void) >>> +{ >>> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >>> + >>> + return !((dev_id & JTAG_DEV_VIDEO_CODEC_MASK) >> >>> JTAG_DEV_VIDEO_CODEC_SHIFT); >>> +} >>> + >>> +static inline int k3_has_canfd(void) >>> +{ >>> + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); >>> + >>> + return (dev_id & JTAG_DEV_CANFD_MASK) >> JTAG_DEV_CANFD_SHIFT; >>> +} >>> + >>> #if defined(CONFIG_SYS_K3_SPL_ATF) && !defined(__ASSEMBLY__) >>> static const u32 put_device_ids[] = {}; >>> -- >>> 2.34.1 >>> >> We should do a proper framework for this that scales multiple devices. > > I agree, this should be reworked afterwards and moved into a > framework. We have literally the same logic for AM62 and AM62Ax. I > also have temperature fixup patches for the AM64 which are identical > again. Would be nice to move most of this logic into a common part. > > Thanks for sending those patches! > > - Daniel > Thanks for reviewing the patches Daniel. We will soon define a common framework and reuse APIs wherever possible. - Aparna >> >>
On 30/04/25 20:50, Bryan Brattlof wrote: > On April 30, 2025 thus sayeth Aparna Patra: >> AM62A SOC is available in multiple variants: >> -CPU cores (Cortex-A) AM62Ax1 (1 core), >> AM62Ax2 (2 cores), AM62Ax4 (4 cores) >> -With and without DSS, CAN-FD & Video-codec support >> >> Remove the relevant FDT nodes by reading the actual configuration >> from the SoC registers, with that change it is possible to have a single >> dts/dtb file handling the different variants at runtime. >> >> Signed-off-by: Aparna Patra <a-patra@ti.com> >> --- >> arch/arm/mach-k3/am62ax/am62a7_fdt.c | 41 +++++++++++++++++++ >> .../arm/mach-k3/include/mach/am62a_hardware.h | 40 ++++++++++++++++++ >> 2 files changed, 81 insertions(+) >> >> diff --git a/arch/arm/mach-k3/am62ax/am62a7_fdt.c b/arch/arm/mach-k3/am62ax/am62a7_fdt.c >> index 7f764ab36b..56fd658202 100644 >> --- a/arch/arm/mach-k3/am62ax/am62a7_fdt.c >> +++ b/arch/arm/mach-k3/am62ax/am62a7_fdt.c >> @@ -8,8 +8,49 @@ >> >> #include "../common_fdt.h" >> >> +static void fdt_fixup_cores_wdt_nodes_am62a(void *blob, int core_nr) >> +{ >> + char node_path[32]; >> + >> + if (core_nr < 1) >> + return; >> + >> + for (; core_nr < 4; core_nr++) { >> + snprintf(node_path, sizeof(node_path), "/cpus/cpu@%d", core_nr); >> + fdt_del_node_path(blob, node_path); >> + snprintf(node_path, sizeof(node_path), "/cpus/cpu-map/cluster0/core%d", core_nr); >> + fdt_del_node_path(blob, node_path); >> + snprintf(node_path, sizeof(node_path), "/bus@f0000/watchdog@e0%d0000", core_nr); >> + fdt_del_node_path(blob, node_path); >> + } >> +} >> + >> +static void fdt_fixup_dss_nodes_am62a(void *blob, bool has_dss) >> +{ >> + if (!has_dss) { >> + fdt_del_node_path(blob, "/bus@f0000/dss@30200000"); >> + fdt_del_node_path(blob, "/bus@f0000/pinctrl@f4000/main-dss0-default-pins"); > So for a little background on the upcoming paragraph[0] > > I completely agree we should remove the dss{} node but don't think it's > wise of us to start trying to unwind the allocation or configuration of > the hardware when the hardware isn't there. > > In my eyes because we can't really trust the names for the memory > carveouts, mailbox and, pinmux nodes as they are just describing their > use and are fairly unstable as the boards age we would need to keep up > with each node's bindings and follow all the pinmux, mboxes, > memory-region phandles via their properties to undo all those nodes > which leaves us making too many opinionated decisions on how best to do > that in my opinion. > > Anywho from that first link I'm trusting Andrew has a plan which from > what I've seen could look something like an overlay[1] But time will > tell. > > ~Bryan > > [0] https://lore.kernel.org/all/20250419150451.v3jgtgp4yisou65u@bryanbrattlof.com/ > [1] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?id=95010a7c186b86f738615231016aa017ac527612 Sure Bryan, I will test without the pinmux fixup, and send out the modified patches. Thanks, Aparna >
diff --git a/arch/arm/mach-k3/am62ax/am62a7_fdt.c b/arch/arm/mach-k3/am62ax/am62a7_fdt.c index 7f764ab36b..56fd658202 100644 --- a/arch/arm/mach-k3/am62ax/am62a7_fdt.c +++ b/arch/arm/mach-k3/am62ax/am62a7_fdt.c @@ -8,8 +8,49 @@ #include "../common_fdt.h" +static void fdt_fixup_cores_wdt_nodes_am62a(void *blob, int core_nr) +{ + char node_path[32]; + + if (core_nr < 1) + return; + + for (; core_nr < 4; core_nr++) { + snprintf(node_path, sizeof(node_path), "/cpus/cpu@%d", core_nr); + fdt_del_node_path(blob, node_path); + snprintf(node_path, sizeof(node_path), "/cpus/cpu-map/cluster0/core%d", core_nr); + fdt_del_node_path(blob, node_path); + snprintf(node_path, sizeof(node_path), "/bus@f0000/watchdog@e0%d0000", core_nr); + fdt_del_node_path(blob, node_path); + } +} + +static void fdt_fixup_dss_nodes_am62a(void *blob, bool has_dss) +{ + if (!has_dss) { + fdt_del_node_path(blob, "/bus@f0000/dss@30200000"); + fdt_del_node_path(blob, "/bus@f0000/pinctrl@f4000/main-dss0-default-pins"); + } +} + +static void fdt_fixup_video_codec_nodes_am62a(void *blob, bool has_video_codec) +{ + if (!has_video_codec) + fdt_del_node_path(blob, "/bus@f0000/video-codec@30210000"); +} + +static void fdt_fixup_canfd_nodes_am62a(void *blob, bool has_canfd) +{ + if (!has_canfd) + fdt_del_node_path(blob, "/bus@f0000/can@20701000"); +} + int ft_system_setup(void *blob, struct bd_info *bd) { + fdt_fixup_cores_wdt_nodes_am62a(blob, k3_get_core_nr()); + fdt_fixup_dss_nodes_am62a(blob, k3_has_dss()); + fdt_fixup_video_codec_nodes_am62a(blob, k3_has_video_codec()); + fdt_fixup_canfd_nodes_am62a(blob, k3_has_canfd()); fdt_fixup_reserved(blob, "tfa", CONFIG_K3_ATF_LOAD_ADDR, 0x80000); fdt_fixup_reserved(blob, "optee", CONFIG_K3_OPTEE_LOAD_ADDR, 0x1800000); diff --git a/arch/arm/mach-k3/include/mach/am62a_hardware.h b/arch/arm/mach-k3/include/mach/am62a_hardware.h index cd61abe018..ab281129d7 100644 --- a/arch/arm/mach-k3/include/mach/am62a_hardware.h +++ b/arch/arm/mach-k3/include/mach/am62a_hardware.h @@ -19,6 +19,18 @@ #define MCU_CTRL_MMR0_BASE 0x04500000 #define WKUP_CTRL_MMR0_BASE 0x43000000 +#define CTRLMMR_WKUP_JTAG_DEVICE_ID (WKUP_CTRL_MMR0_BASE + 0x18) +#define JTAG_DEV_CORE_NR_MASK GENMASK(19, 18) +#define JTAG_DEV_CORE_NR_SHIFT 18 +#define JTAG_DEV_CANFD_MASK BIT(15) +#define JTAG_DEV_CANFD_SHIFT 15 +#define JTAG_DEV_VIDEO_CODEC_MASK BIT(14) +#define JTAG_DEV_VIDEO_CODEC_SHIFT 14 +#define JTAG_DEV_DSS_MASK GENMASK(16, 13) +#define JTAG_DEV_DSS_SHIFT 13 + +#define JTAG_DEV_HAS_DSS_VALUE 0xD + #define CTRLMMR_MAIN_DEVSTAT (WKUP_CTRL_MMR0_BASE + 0x30) #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK GENMASK(6, 3) #define MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT 3 @@ -86,6 +98,34 @@ #define TI_SRAM_SCRATCH_BOARD_EEPROM_START 0x70000001 #endif /* CONFIG_CPU_V7R */ +static inline int k3_get_core_nr(void) +{ + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); + + return ((dev_id & JTAG_DEV_CORE_NR_MASK) >> JTAG_DEV_CORE_NR_SHIFT) + 1; +} + +static inline int k3_has_dss(void) +{ + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); + + return (JTAG_DEV_HAS_DSS_VALUE == ((dev_id & JTAG_DEV_DSS_MASK) >> JTAG_DEV_DSS_SHIFT)); +} + +static inline int k3_has_video_codec(void) +{ + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); + + return !((dev_id & JTAG_DEV_VIDEO_CODEC_MASK) >> JTAG_DEV_VIDEO_CODEC_SHIFT); +} + +static inline int k3_has_canfd(void) +{ + u32 dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID); + + return (dev_id & JTAG_DEV_CANFD_MASK) >> JTAG_DEV_CANFD_SHIFT; +} + #if defined(CONFIG_SYS_K3_SPL_ATF) && !defined(__ASSEMBLY__) static const u32 put_device_ids[] = {};
AM62A SOC is available in multiple variants: -CPU cores (Cortex-A) AM62Ax1 (1 core), AM62Ax2 (2 cores), AM62Ax4 (4 cores) -With and without DSS, CAN-FD & Video-codec support Remove the relevant FDT nodes by reading the actual configuration from the SoC registers, with that change it is possible to have a single dts/dtb file handling the different variants at runtime. Signed-off-by: Aparna Patra <a-patra@ti.com> --- arch/arm/mach-k3/am62ax/am62a7_fdt.c | 41 +++++++++++++++++++ .../arm/mach-k3/include/mach/am62a_hardware.h | 40 ++++++++++++++++++ 2 files changed, 81 insertions(+)