[PATCH 0/3] fdt_support: improve board_fdt_chosen_bootargs() for flexibility
This series consists of three patches.
The first patch modifies the function documentation style in the include/fdt_support.h file to comply with kernel-doc requirements.
The second patch modifies the board_fdt_chosen_bootargs() function to return a const char* type. This change clarifies to the caller that the returned string should neither be freed nor modified. It aligns with the existing fdt_setprop() function, which already utilizes a const char* parameter. This promotes consistency within the codebase and enhances code safety by preventing unintended modifications to the returned string.
The third patch addresses the need for flexibility in providing kernel command line arguments (bootargs) for different kernel images within the same U-Boot environment. It introduces a read-only (RO) fdt_property argument to the board_fdt_chosen_bootargs() function, allowing access to the original chosen/bootargs data. This is crucial for scenarios where different kernel versions require distinct console setups (e.g., ttyS0 for vendor kernels and ttyAML0 for upstream kernels). By enabling board developers to either merge or replace the original bootargs, this patch enhances the configurability of U-Boot for various kernel images without relying on outdated configurations like CMDLINE_EXTEND.
CI/CD results: https://github.com/u-boot/u-boot/pull/716/checks
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com --- Dmitry Rokosov (3): include/fdt_support: fix docstyle to comply with kernel-doc requirements fdt_support: board_fdt_chosen_bootargs() should return const char* common: fdt: hand over original fdt bootargs into board chosen handler
boot/fdt_support.c | 7 +- include/fdt_support.h | 173 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 107 insertions(+), 73 deletions(-) --- base-commit: 69bd83568c57813cd23bc2d100c066a17e7e349d change-id: 20241219-board_fdt_chosen_bootargs_improvements-7646b2029ba7
Best regards,
No errors from kernel-doc with this patch applied: $ ./scripts/kernel-doc -v -none include/fdt_support.h include/fdt_support.h:17: info: Scanning doc for arch_fixup_fdt include/fdt_support.h:37: info: Scanning doc for fdt_root include/fdt_support.h:48: info: Scanning doc for fdt_chosen include/fdt_support.h:59: info: Scanning doc for fdt_initrd include/fdt_support.h:100: info: Scanning doc for fdt_fixup_memory include/fdt_support.h:115: info: Scanning doc for fdt_fixup_memory_banks include/fdt_support.h:148: info: Scanning doc for fdt_fixup_display include/fdt_support.h:176: info: Scanning doc for fdt_record_loadable include/fdt_support.h:205: info: Scanning doc for ft_board_setup include/fdt_support.h:218: info: Scanning doc for board_rng_seed include/fdt_support.h:231: info: Scanning doc for board_fdt_chosen_bootargs include/fdt_support.h:251: info: Scanning doc for ft_system_setup include/fdt_support.h:266: info: Scanning doc for fdt_shrink_to_minimum include/fdt_support.h:301: info: Scanning doc for fdt_copy_fixed_partitions include/fdt_support.h:314: info: Scanning doc for fdt_translate_address include/fdt_support.h:327: info: Scanning doc for fdt_translate_dma_address include/fdt_support.h:342: info: Scanning doc for fdt_get_dma_range include/fdt_support.h:464: info: Scanning doc for fdt_get_cells_len include/fdt_support.h:480: info: Scanning doc for fdtdec_get_child_count include/fdt_support.h:500: info: Scanning doc for fdt_kaslrseed
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com --- include/fdt_support.h | 169 +++++++++++++++++++++++++++++--------------------- 1 file changed, 100 insertions(+), 69 deletions(-)
diff --git a/include/fdt_support.h b/include/fdt_support.h index 9447a64e060da4c4f6e1c1b509dd477c29e040f4..ed4f5ba26c19b2d0afcc43c46bdee3c211c0441a 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -14,11 +14,12 @@ #include <abuf.h>
/** - * arch_fixup_fdt() - Write arch-specific information to fdt + * arch_fixup_fdt() - write arch-specific information to fdt + * + * @blob: FDT blob to write to * * Defined in arch/$(ARCH)/lib/bootm-fdt.c * - * @blob: FDT blob to write to * Return: 0 if ok, or -ve FDT_ERR_... on failure */ int arch_fixup_fdt(void *blob); @@ -33,27 +34,33 @@ u32 fdt_getprop_u32_default(const void *fdt, const char *path, const char *prop, const u32 dflt);
/** - * Add data to the root of the FDT before booting the OS. + * fdt_root() - add data to the root of the FDT before booting the OS + * + * @fdt: FDT address in memory * * See doc/device-tree-bindings/root.txt * - * @param fdt FDT address in memory * Return: 0 if ok, or -FDT_ERR_... on error */ int fdt_root(void *fdt);
/** - * Add chosen data the FDT before booting the OS. + * fdt_chosen() - add chosen data the FDT before booting the OS + * + * @fdt: FDT address in memory * * In particular, this adds the kernel command line (bootargs) to the FDT. * - * @param fdt FDT address in memory * Return: 0 if ok, or -FDT_ERR_... on error */ int fdt_chosen(void *fdt);
/** - * Add initrd information to the FDT before booting the OS. + * fdt_initrd() - add initrd information to the FDT before booting the OS + * + * @fdt: Pointer to FDT in memory + * @initrd_start: Start of ramdisk + * @initrd_end: End of ramdisk * * Adds linux,initrd-start and linux,initrd-end properties to the /chosen node, * creating it if necessary. @@ -63,9 +70,6 @@ int fdt_chosen(void *fdt); * * If @initrd_start == @initrd_end this function does nothing and returns 0. * - * @fdt: Pointer to FDT in memory - * @initrd_start: Start of ramdisk - * @initrd_end: End of ramdisk * Return: 0 if ok, or -FDT_ERR_... on error */ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end); @@ -93,29 +97,35 @@ void do_fixup_by_compat(void *fdt, const char *compat, void do_fixup_by_compat_u32(void *fdt, const char *compat, const char *prop, u32 val, int create); /** + * fdt_fixup_memory() - setup the memory node in the DT + * + * @blob: FDT blob to update + * @start: Begin of DRAM mapping in physical memory + * @size: Size of the single memory bank + * * Setup the memory node in the DT. Creates one if none was existing before. * Calls fdt_fixup_memory_banks() to populate a single reg pair covering the * whole memory. * - * @param blob FDT blob to update - * @param start Begin of DRAM mapping in physical memory - * @param size Size of the single memory bank * Return: 0 if ok, or -1 or -FDT_ERR_... on error */ int fdt_fixup_memory(void *blob, u64 start, u64 size);
/** + * fdt_fixup_memory_banks() - fill the DT mem node with multiple memory banks + * + * @blob: FDT blob to update + * @start: Array of size <banks> to hold the start addresses. + * @size: Array of size <banks> to hold the size of each region. + * @banks: Number of memory banks to create. If 0, the reg property + * will be left untouched. + * * Fill the DT memory node with multiple memory banks. * Creates the node if none was existing before. * If banks is 0, it will not touch the existing reg property. This allows * boards to not mess with the existing DT setup, which may have been * filled in properly before. * - * @param blob FDT blob to update - * @param start Array of size <banks> to hold the start addresses. - * @param size Array of size <banks> to hold the size of each region. - * @param banks Number of memory banks to create. If 0, the reg - * property will be left untouched. * Return: 0 if ok, or -1 or -FDT_ERR_... on error */ #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY @@ -135,14 +145,17 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, void fdt_fixup_qe_firmware(void *fdt);
/** + * fdt_fixup_display() - update native-mode property of display-timings + * + * @blob: FDT blob to update + * @path: path within dt + * @display: name of display timing to match + * * Update native-mode property of display-timings node to the phandle * of the timings matching a display by name (case insensitive). * * see kernel Documentation/devicetree/bindings/video/display-timing.txt * - * @param blob FDT blob to update - * @param path path within dt - * @param display name of display timing to match * Return: 0 if ok, or -FDT_ERR_... on error */ int fdt_fixup_display(void *blob, const char *path, const char *display); @@ -160,18 +173,21 @@ static inline void fdt_fixup_crypto_node(void *blob, int sec_rev) {} #endif
/** + * fdt_record_loadable() - record info about a loadable in /fit-images + * + * @blob: FDT blob to update + * @index: index of this loadable + * @name: name of the loadable + * @load_addr: address the loadable was loaded to + * @size: number of bytes loaded + * @entry_point: entry point (if specified, otherwise pass -1) + * @type: type (if specified, otherwise pass NULL) + * @os: os-type (if specified, otherwise pass NULL) + * @arch: architecture (if specified, otherwise pass NULL) + * * Record information about a processed loadable in /fit-images (creating * /fit-images if necessary). * - * @param blob FDT blob to update - * @param index index of this loadable - * @param name name of the loadable - * @param load_addr address the loadable was loaded to - * @param size number of bytes loaded - * @param entry_point entry point (if specified, otherwise pass -1) - * @param type type (if specified, otherwise pass NULL) - * @param os os-type (if specified, otherwise pass NULL) - * @param arch architecture (if specified, otherwise pass NULL) * Return: 0 if ok, or -1 or -FDT_ERR_... on error */ int fdt_record_loadable(void *blob, u32 index, const char *name, @@ -186,37 +202,39 @@ int fdt_pci_dma_ranges(void *blob, int phb_off, struct pci_controller *hose); int fdt_find_or_add_subnode(void *fdt, int parentoffset, const char *name);
/** - * Add board-specific data to the FDT before booting the OS. + * ft_board_setup() - add board-specific data to the FDT before booting the OS + * + * @blob: FDT blob to update + * @bd: Pointer to board data * * Use CONFIG_SYS_FDT_PAD to ensure there is sufficient space. * This function is called if CONFIG_OF_BOARD_SETUP is defined * - * @param blob FDT blob to update - * @param bd Pointer to board data * Return: 0 if ok, or -FDT_ERR_... on error */ int ft_board_setup(void *blob, struct bd_info *bd);
/** - * board_rng_seed() - Provide a seed to be passed via /chosen/rng-seed + * board_rng_seed() - provide a seed to be passed via /chosen/rng-seed + * + * @buf: a struct abuf for returning the seed and its size. * * This function is called if CONFIG_BOARD_RNG_SEED is set, and must * be provided by the board. It should return, via @buf, some suitable * seed value to pass to the kernel. Seed size could be set in a decimal * environment variable rng_seed_size and it defaults to 64 bytes. * - * @param buf A struct abuf for returning the seed and its size. - * @return 0 if ok, negative on error. + * Return: 0 if ok, negative on error. */ int board_rng_seed(struct abuf *buf);
/** - * board_fdt_chosen_bootargs() - Arbitrarily amend fdt kernel command line + * board_fdt_chosen_bootargs() - arbitrarily amend fdt kernel command line * * This is used for late modification of kernel command line arguments just * before they are added into the /chosen node in flat device tree. * - * @return: pointer to kernel command line arguments in memory + * Return: pointer to kernel command line arguments in memory */ char *board_fdt_chosen_bootargs(void);
@@ -229,13 +247,14 @@ char *board_fdt_chosen_bootargs(void); void ft_board_setup_ex(void *blob, struct bd_info *bd);
/** - * Add system-specific data to the FDT before booting the OS. + * ft_system_setup() - add system-specific data to the FDT before booting the OS + * + * @blob: FDT blob to update + * @bd: pointer to board data * * Use CONFIG_SYS_FDT_PAD to ensure there is sufficient space. * This function is called if CONFIG_OF_SYSTEM_SETUP is defined * - * @param blob FDT blob to update - * @param bd Pointer to board data * Return: 0 if ok, or -FDT_ERR_... on error */ int ft_system_setup(void *blob, struct bd_info *bd); @@ -245,6 +264,9 @@ void set_working_fdt_addr(ulong addr); /** * fdt_shrink_to_minimum() - shrink FDT while allowing for some margin * + * @blob: FDT blob to update + * @extrasize: additional bytes needed + * * Shrink down the given blob to 'minimum' size + some extrasize. * * The new size is enough to hold the existing contents plus @extrasize bytes, @@ -254,8 +276,6 @@ void set_working_fdt_addr(ulong addr); * If there is an existing memory reservation for @blob in the FDT, it is * updated for the new size. * - * @param blob FDT blob to update - * @param extrasize additional bytes needed * Return: 0 if ok, or -FDT_ERR_... on error */ int fdt_shrink_to_minimum(void *blob, uint extrasize); @@ -277,9 +297,12 @@ static inline void fdt_fixup_mtdparts(void *fdt, #endif
/** - * copy the fixed-partition nodes from U-Boot device tree to external blob + * fdt_copy_fixed_partitions() - copy the fixed-partition nodes + * + * @blob: FDT blob to update + * + * Copy the fixed-partition nodes from U-Boot device tree to external blob * - * @param blob FDT blob to update * Return: 0 if ok, or non-zero on error */ int fdt_copy_fixed_partitions(void *blob); @@ -287,39 +310,45 @@ int fdt_copy_fixed_partitions(void *blob); void fdt_del_node_and_alias(void *blob, const char *alias);
/** - * Translate an address from the DT into a CPU physical address + * fdt_translate_address() - translate an addr from the DT into a CPU phys addr + * + * @blob: pointer to device tree blob + * @node_offset: node DT offset + * @in_addr: pointer to the address to translate * * The translation relies on the "ranges" property. * - * @param blob Pointer to device tree blob - * @param node_offset Node DT offset - * @param in_addr Pointer to the address to translate * Return: translated address or OF_BAD_ADDR on error */ u64 fdt_translate_address(const void *blob, int node_offset, const __be32 *in_addr); /** - * Translate a DMA address from the DT into a CPU physical address + * fdt_translate_dma_address() - translate a DMA address to a CPU phys address * + * @blob: pointer to device tree blob + * @node_offset: node DT offset + * @in_addr: pointer to the DMA address to translate + * + * Translate a DMA address from the DT into a CPU physical address. * The translation relies on the "dma-ranges" property. * - * @param blob Pointer to device tree blob - * @param node_offset Node DT offset - * @param in_addr Pointer to the DMA address to translate * Return: translated DMA address or OF_BAD_ADDR on error */ u64 fdt_translate_dma_address(const void *blob, int node_offset, const __be32 *in_addr);
/** + * fdt_get_dma_range() - get DMA ranges to perform bus/cpu translations + * + * @blob: pointer to device tree blob + * @node_offset: node DT offset + * @cpu: pointer to variable storing the range's cpu address + * @bus: pointer to variable storing the range's bus address + * @size: pointer to variable storing the range's size + * * Get DMA ranges for a specifc node, this is useful to perform bus->cpu and * cpu->bus address translations * - * @param blob Pointer to device tree blob - * @param node_offset Node DT offset - * @param cpu Pointer to variable storing the range's cpu address - * @param bus Pointer to variable storing the range's bus address - * @param size Pointer to variable storing the range's size * Return: translated DMA address or OF_BAD_ADDR on error */ int fdt_get_dma_range(const void *blob, int node_offset, phys_addr_t *cpu, @@ -431,12 +460,12 @@ int fdt_overlay_apply_verbose(void *fdt, void *fdto); int fdt_valid(struct fdt_header **blobp);
/** - * fdt_get_cells_len() - Get the length of a type of cell in top-level nodes + * fdt_get_cells_len() - get the length of a type of cell in top-level nodes * - * Returns the length of the cell type in bytes (4 or 8). + * @blob: pointer to device tree blob + * @nr_cells_name: name to lookup, e.g. "#address-cells" * - * @blob: Pointer to device tree blob - * @nr_cells_name: Name to lookup, e.g. "#address-cells" + * Return: the length of the cell type in bytes (4 or 8). */ int fdt_get_cells_len(const void *blob, char *nr_cells_name);
@@ -446,11 +475,12 @@ int fdt_get_cells_len(const void *blob, char *nr_cells_name); int fdtdec_get_int(const void *blob, int node, const char *prop_name, int default_val);
-/* - * Count child nodes of one parent node. +/** + * fdtdec_get_child_count() - count child nodes of one parent node + * + * @blob: FDT blob + * @node: parent node * - * @param blob FDT blob - * @param node parent node * Return: number of child node; 0 if there is not child node */ int fdtdec_get_child_count(const void *blob, int node); @@ -468,9 +498,10 @@ void fdt_fixup_pstore(void *blob); /** * fdt_kaslrseed() - create a 'kaslr-seed' node in chosen * - * @blob: fdt blob - * @overwrite: do not overwrite existing non-zero node unless true - * Return: 0 if OK, -ve on error + * @blob: fdt blob + * @overwrite: do not overwrite existing non-zero node unless true + * + * Return: 0 if OK, -ve on error */ int fdt_kaslrseed(void *blob, bool overwrite);
Am 19. Dezember 2024 22:42:08 MEZ schrieb Dmitry Rokosov ddrokosov@salutedevices.com:
No errors from kernel-doc with this patch applied: $ ./scripts/kernel-doc -v -none include/fdt_support.h include/fdt_support.h:17: info: Scanning doc for arch_fixup_fdt include/fdt_support.h:37: info: Scanning doc for fdt_root include/fdt_support.h:48: info: Scanning doc for fdt_chosen include/fdt_support.h:59: info: Scanning doc for fdt_initrd include/fdt_support.h:100: info: Scanning doc for fdt_fixup_memory include/fdt_support.h:115: info: Scanning doc for fdt_fixup_memory_banks include/fdt_support.h:148: info: Scanning doc for fdt_fixup_display include/fdt_support.h:176: info: Scanning doc for fdt_record_loadable include/fdt_support.h:205: info: Scanning doc for ft_board_setup include/fdt_support.h:218: info: Scanning doc for board_rng_seed include/fdt_support.h:231: info: Scanning doc for board_fdt_chosen_bootargs include/fdt_support.h:251: info: Scanning doc for ft_system_setup include/fdt_support.h:266: info: Scanning doc for fdt_shrink_to_minimum include/fdt_support.h:301: info: Scanning doc for fdt_copy_fixed_partitions include/fdt_support.h:314: info: Scanning doc for fdt_translate_address include/fdt_support.h:327: info: Scanning doc for fdt_translate_dma_address include/fdt_support.h:342: info: Scanning doc for fdt_get_dma_range include/fdt_support.h:464: info: Scanning doc for fdt_get_cells_len include/fdt_support.h:480: info: Scanning doc for fdtdec_get_child_count include/fdt_support.h:500: info: Scanning doc for fdt_kaslrseed
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
Thanks for eliminating @param.
Acked-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/fdt_support.h | 169 +++++++++++++++++++++++++++++--------------------- 1 file changed, 100 insertions(+), 69 deletions(-)
diff --git a/include/fdt_support.h b/include/fdt_support.h index 9447a64e060da4c4f6e1c1b509dd477c29e040f4..ed4f5ba26c19b2d0afcc43c46bdee3c211c0441a 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -14,11 +14,12 @@ #include <abuf.h>
/**
- arch_fixup_fdt() - Write arch-specific information to fdt
- arch_fixup_fdt() - write arch-specific information to fdt
- @blob: FDT blob to write to
- Defined in arch/$(ARCH)/lib/bootm-fdt.c
- @blob: FDT blob to write to
- Return: 0 if ok, or -ve FDT_ERR_... on failure
*/ int arch_fixup_fdt(void *blob); @@ -33,27 +34,33 @@ u32 fdt_getprop_u32_default(const void *fdt, const char *path, const char *prop, const u32 dflt);
/**
- Add data to the root of the FDT before booting the OS.
- fdt_root() - add data to the root of the FDT before booting the OS
- @fdt: FDT address in memory
- See doc/device-tree-bindings/root.txt
- @param fdt FDT address in memory
- Return: 0 if ok, or -FDT_ERR_... on error
*/ int fdt_root(void *fdt);
/**
- Add chosen data the FDT before booting the OS.
- fdt_chosen() - add chosen data the FDT before booting the OS
- @fdt: FDT address in memory
- In particular, this adds the kernel command line (bootargs) to the FDT.
- @param fdt FDT address in memory
- Return: 0 if ok, or -FDT_ERR_... on error
*/ int fdt_chosen(void *fdt);
/**
- Add initrd information to the FDT before booting the OS.
- fdt_initrd() - add initrd information to the FDT before booting the OS
- @fdt: Pointer to FDT in memory
- @initrd_start: Start of ramdisk
- @initrd_end: End of ramdisk
- Adds linux,initrd-start and linux,initrd-end properties to the /chosen node,
- creating it if necessary.
@@ -63,9 +70,6 @@ int fdt_chosen(void *fdt);
- If @initrd_start == @initrd_end this function does nothing and returns 0.
- @fdt: Pointer to FDT in memory
- @initrd_start: Start of ramdisk
- @initrd_end: End of ramdisk
- Return: 0 if ok, or -FDT_ERR_... on error
*/ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end); @@ -93,29 +97,35 @@ void do_fixup_by_compat(void *fdt, const char *compat, void do_fixup_by_compat_u32(void *fdt, const char *compat, const char *prop, u32 val, int create); /**
- fdt_fixup_memory() - setup the memory node in the DT
- @blob: FDT blob to update
- @start: Begin of DRAM mapping in physical memory
- @size: Size of the single memory bank
- Setup the memory node in the DT. Creates one if none was existing before.
- Calls fdt_fixup_memory_banks() to populate a single reg pair covering the
- whole memory.
- @param blob FDT blob to update
- @param start Begin of DRAM mapping in physical memory
- @param size Size of the single memory bank
- Return: 0 if ok, or -1 or -FDT_ERR_... on error
*/ int fdt_fixup_memory(void *blob, u64 start, u64 size);
/**
- fdt_fixup_memory_banks() - fill the DT mem node with multiple memory banks
- @blob: FDT blob to update
- @start: Array of size <banks> to hold the start addresses.
- @size: Array of size <banks> to hold the size of each region.
- @banks: Number of memory banks to create. If 0, the reg property
will be left untouched.
- Fill the DT memory node with multiple memory banks.
- Creates the node if none was existing before.
- If banks is 0, it will not touch the existing reg property. This allows
- boards to not mess with the existing DT setup, which may have been
- filled in properly before.
- @param blob FDT blob to update
- @param start Array of size <banks> to hold the start addresses.
- @param size Array of size <banks> to hold the size of each region.
- @param banks Number of memory banks to create. If 0, the reg
property will be left untouched.
- Return: 0 if ok, or -1 or -FDT_ERR_... on error
*/ #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY @@ -135,14 +145,17 @@ int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, void fdt_fixup_qe_firmware(void *fdt);
/**
- fdt_fixup_display() - update native-mode property of display-timings
- @blob: FDT blob to update
- @path: path within dt
- @display: name of display timing to match
- Update native-mode property of display-timings node to the phandle
- of the timings matching a display by name (case insensitive).
- see kernel Documentation/devicetree/bindings/video/display-timing.txt
- @param blob FDT blob to update
- @param path path within dt
- @param display name of display timing to match
- Return: 0 if ok, or -FDT_ERR_... on error
*/ int fdt_fixup_display(void *blob, const char *path, const char *display); @@ -160,18 +173,21 @@ static inline void fdt_fixup_crypto_node(void *blob, int sec_rev) {} #endif
/**
- fdt_record_loadable() - record info about a loadable in /fit-images
- @blob: FDT blob to update
- @index: index of this loadable
- @name: name of the loadable
- @load_addr: address the loadable was loaded to
- @size: number of bytes loaded
- @entry_point: entry point (if specified, otherwise pass -1)
- @type: type (if specified, otherwise pass NULL)
- @os: os-type (if specified, otherwise pass NULL)
- @arch: architecture (if specified, otherwise pass NULL)
- Record information about a processed loadable in /fit-images (creating
- /fit-images if necessary).
- @param blob FDT blob to update
- @param index index of this loadable
- @param name name of the loadable
- @param load_addr address the loadable was loaded to
- @param size number of bytes loaded
- @param entry_point entry point (if specified, otherwise pass -1)
- @param type type (if specified, otherwise pass NULL)
- @param os os-type (if specified, otherwise pass NULL)
- @param arch architecture (if specified, otherwise pass NULL)
- Return: 0 if ok, or -1 or -FDT_ERR_... on error
*/ int fdt_record_loadable(void *blob, u32 index, const char *name, @@ -186,37 +202,39 @@ int fdt_pci_dma_ranges(void *blob, int phb_off, struct pci_controller *hose); int fdt_find_or_add_subnode(void *fdt, int parentoffset, const char *name);
/**
- Add board-specific data to the FDT before booting the OS.
- ft_board_setup() - add board-specific data to the FDT before booting the OS
- @blob: FDT blob to update
- @bd: Pointer to board data
- Use CONFIG_SYS_FDT_PAD to ensure there is sufficient space.
- This function is called if CONFIG_OF_BOARD_SETUP is defined
- @param blob FDT blob to update
- @param bd Pointer to board data
- Return: 0 if ok, or -FDT_ERR_... on error
*/ int ft_board_setup(void *blob, struct bd_info *bd);
/**
- board_rng_seed() - Provide a seed to be passed via /chosen/rng-seed
- board_rng_seed() - provide a seed to be passed via /chosen/rng-seed
- @buf: a struct abuf for returning the seed and its size.
- This function is called if CONFIG_BOARD_RNG_SEED is set, and must
- be provided by the board. It should return, via @buf, some suitable
- seed value to pass to the kernel. Seed size could be set in a decimal
- environment variable rng_seed_size and it defaults to 64 bytes.
- @param buf A struct abuf for returning the seed and its size.
- @return 0 if ok, negative on error.
- Return: 0 if ok, negative on error.
*/ int board_rng_seed(struct abuf *buf);
/**
- board_fdt_chosen_bootargs() - Arbitrarily amend fdt kernel command line
- board_fdt_chosen_bootargs() - arbitrarily amend fdt kernel command line
- This is used for late modification of kernel command line arguments just
- before they are added into the /chosen node in flat device tree.
- @return: pointer to kernel command line arguments in memory
- Return: pointer to kernel command line arguments in memory
*/ char *board_fdt_chosen_bootargs(void);
@@ -229,13 +247,14 @@ char *board_fdt_chosen_bootargs(void); void ft_board_setup_ex(void *blob, struct bd_info *bd);
/**
- Add system-specific data to the FDT before booting the OS.
- ft_system_setup() - add system-specific data to the FDT before booting the OS
- @blob: FDT blob to update
- @bd: pointer to board data
- Use CONFIG_SYS_FDT_PAD to ensure there is sufficient space.
- This function is called if CONFIG_OF_SYSTEM_SETUP is defined
- @param blob FDT blob to update
- @param bd Pointer to board data
- Return: 0 if ok, or -FDT_ERR_... on error
*/ int ft_system_setup(void *blob, struct bd_info *bd); @@ -245,6 +264,9 @@ void set_working_fdt_addr(ulong addr); /**
- fdt_shrink_to_minimum() - shrink FDT while allowing for some margin
- @blob: FDT blob to update
- @extrasize: additional bytes needed
- Shrink down the given blob to 'minimum' size + some extrasize.
- The new size is enough to hold the existing contents plus @extrasize bytes,
@@ -254,8 +276,6 @@ void set_working_fdt_addr(ulong addr);
- If there is an existing memory reservation for @blob in the FDT, it is
- updated for the new size.
- @param blob FDT blob to update
- @param extrasize additional bytes needed
- Return: 0 if ok, or -FDT_ERR_... on error
*/ int fdt_shrink_to_minimum(void *blob, uint extrasize); @@ -277,9 +297,12 @@ static inline void fdt_fixup_mtdparts(void *fdt, #endif
/**
- copy the fixed-partition nodes from U-Boot device tree to external blob
- fdt_copy_fixed_partitions() - copy the fixed-partition nodes
- @blob: FDT blob to update
- Copy the fixed-partition nodes from U-Boot device tree to external blob
- @param blob FDT blob to update
- Return: 0 if ok, or non-zero on error
*/ int fdt_copy_fixed_partitions(void *blob); @@ -287,39 +310,45 @@ int fdt_copy_fixed_partitions(void *blob); void fdt_del_node_and_alias(void *blob, const char *alias);
/**
- Translate an address from the DT into a CPU physical address
- fdt_translate_address() - translate an addr from the DT into a CPU phys addr
- @blob: pointer to device tree blob
- @node_offset: node DT offset
- @in_addr: pointer to the address to translate
- The translation relies on the "ranges" property.
- @param blob Pointer to device tree blob
- @param node_offset Node DT offset
- @param in_addr Pointer to the address to translate
- Return: translated address or OF_BAD_ADDR on error
*/ u64 fdt_translate_address(const void *blob, int node_offset, const __be32 *in_addr); /**
- Translate a DMA address from the DT into a CPU physical address
- fdt_translate_dma_address() - translate a DMA address to a CPU phys address
- @blob: pointer to device tree blob
- @node_offset: node DT offset
- @in_addr: pointer to the DMA address to translate
- Translate a DMA address from the DT into a CPU physical address.
- The translation relies on the "dma-ranges" property.
- @param blob Pointer to device tree blob
- @param node_offset Node DT offset
- @param in_addr Pointer to the DMA address to translate
- Return: translated DMA address or OF_BAD_ADDR on error
*/ u64 fdt_translate_dma_address(const void *blob, int node_offset, const __be32 *in_addr);
/**
- fdt_get_dma_range() - get DMA ranges to perform bus/cpu translations
- @blob: pointer to device tree blob
- @node_offset: node DT offset
- @cpu: pointer to variable storing the range's cpu address
- @bus: pointer to variable storing the range's bus address
- @size: pointer to variable storing the range's size
- Get DMA ranges for a specifc node, this is useful to perform bus->cpu and
- cpu->bus address translations
- @param blob Pointer to device tree blob
- @param node_offset Node DT offset
- @param cpu Pointer to variable storing the range's cpu address
- @param bus Pointer to variable storing the range's bus address
- @param size Pointer to variable storing the range's size
- Return: translated DMA address or OF_BAD_ADDR on error
*/ int fdt_get_dma_range(const void *blob, int node_offset, phys_addr_t *cpu, @@ -431,12 +460,12 @@ int fdt_overlay_apply_verbose(void *fdt, void *fdto); int fdt_valid(struct fdt_header **blobp);
/**
- fdt_get_cells_len() - Get the length of a type of cell in top-level nodes
- fdt_get_cells_len() - get the length of a type of cell in top-level nodes
- Returns the length of the cell type in bytes (4 or 8).
- @blob: pointer to device tree blob
- @nr_cells_name: name to lookup, e.g. "#address-cells"
- @blob: Pointer to device tree blob
- @nr_cells_name: Name to lookup, e.g. "#address-cells"
- Return: the length of the cell type in bytes (4 or 8).
*/ int fdt_get_cells_len(const void *blob, char *nr_cells_name);
@@ -446,11 +475,12 @@ int fdt_get_cells_len(const void *blob, char *nr_cells_name); int fdtdec_get_int(const void *blob, int node, const char *prop_name, int default_val);
-/*
- Count child nodes of one parent node.
+/**
- fdtdec_get_child_count() - count child nodes of one parent node
- @blob: FDT blob
- @node: parent node
- @param blob FDT blob
- @param node parent node
- Return: number of child node; 0 if there is not child node
*/ int fdtdec_get_child_count(const void *blob, int node); @@ -468,9 +498,10 @@ void fdt_fixup_pstore(void *blob); /**
- fdt_kaslrseed() - create a 'kaslr-seed' node in chosen
- @blob: fdt blob
- @overwrite: do not overwrite existing non-zero node unless true
- Return: 0 if OK, -ve on error
- @blob: fdt blob
- @overwrite: do not overwrite existing non-zero node unless true
- Return: 0 if OK, -ve on error
*/ int fdt_kaslrseed(void *blob, bool overwrite);
It should be structured this way to demonstrate to the caller that freeing the return value is unnecessary and that the caller cannot modify it. The function fdt_setprop() includes a parameter with a const char* prototype, so it is better to use the const qualifier.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com --- boot/fdt_support.c | 4 ++-- include/fdt_support.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2392027d40ba292a4cd714c7f23497e9879ac454..2c39b2dd14b7c447bc4c7c8fad64e22cb00e88e0 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -321,7 +321,7 @@ int fdt_kaslrseed(void *fdt, bool overwrite) * board_fdt_chosen_bootargs - boards may override this function to use * alternative kernel command line arguments */ -__weak char *board_fdt_chosen_bootargs(void) +__weak const char *board_fdt_chosen_bootargs(void) { return env_get("bootargs"); } @@ -331,7 +331,7 @@ int fdt_chosen(void *fdt) struct abuf buf = {}; int nodeoffset; int err; - char *str; /* used to set string properties */ + const char *str; /* used to set string properties */
err = fdt_check_header(fdt); if (err < 0) { diff --git a/include/fdt_support.h b/include/fdt_support.h index ed4f5ba26c19b2d0afcc43c46bdee3c211c0441a..f481881ce640797e0f6204d599f008d0e5dc2bf7 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -236,7 +236,7 @@ int board_rng_seed(struct abuf *buf); * * Return: pointer to kernel command line arguments in memory */ -char *board_fdt_chosen_bootargs(void); +const char *board_fdt_chosen_bootargs(void);
/* * The keystone2 SOC requires all 32 bit aliased addresses to be converted
On 12/19/24 22:42, Dmitry Rokosov wrote:
It should be structured this way to demonstrate to the caller that freeing the return value is unnecessary and that the caller cannot modify it. The function fdt_setprop() includes a parameter with a const char* prototype, so it is better to use the const qualifier.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
boot/fdt_support.c | 4 ++-- include/fdt_support.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2392027d40ba292a4cd714c7f23497e9879ac454..2c39b2dd14b7c447bc4c7c8fad64e22cb00e88e0 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -321,7 +321,7 @@ int fdt_kaslrseed(void *fdt, bool overwrite)
- board_fdt_chosen_bootargs - boards may override this function to use
alternative kernel command line arguments*/ -__weak char *board_fdt_chosen_bootargs(void) +__weak const char *board_fdt_chosen_bootargs(void) { return env_get("bootargs"); } @@ -331,7 +331,7 @@ int fdt_chosen(void *fdt) struct abuf buf = {}; int nodeoffset; int err;
- char *str; /* used to set string properties */
const char *str; /* used to set string properties */
err = fdt_check_header(fdt); if (err < 0) {
diff --git a/include/fdt_support.h b/include/fdt_support.h index ed4f5ba26c19b2d0afcc43c46bdee3c211c0441a..f481881ce640797e0f6204d599f008d0e5dc2bf7 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -236,7 +236,7 @@ int board_rng_seed(struct abuf *buf);
- Return: pointer to kernel command line arguments in memory
*/ -char *board_fdt_chosen_bootargs(void); +const char *board_fdt_chosen_bootargs(void);
/*
- The keystone2 SOC requires all 32 bit aliased addresses to be converted
Sometimes, it is necessary to provide an additional bootargs string to the kernel command line.
We have a real scenario where one U-Boot blob needs to boot several kernel images: the vendor-patched kernel image and the latest upstream kernel image. The Amlogic (Meson architecture) tty driver has different tty suffixes in these kernels: the vendor uses 'ttySx', while the upstream implementation uses 'ttyAMLx'. The initial console setup is provided to the kernel using the kernel command line (bootargs). For the vendor kernel, we should use 'console=ttyS0,115200', while for the upstream kernel, it must be 'console=ttyAML0,115200'. This means we have to use different command line strings depending on the kernel version.
To resolve this issue, we cannot use the CMDLINE_EXTEND kernel configuration because it is considered legacy and is not supported for the arm64 architecture. CMDLINE_EXTEND is outdated primarily because we can provide additional command line strings through the 'chosen/bootargs' FDT node. However, U-Boot uses this node to inject the U-Boot bootargs environment variable content, which results in U-Boot silently overriding all data in the 'chosen/bootargs' node. While we do have the board_fdt_chosen_bootargs() board hook to address such issues, this function lacks any FDT context, such as the original value of the 'chosen/bootargs' node.
This patch introduces a read-only (RO) fdt_property argument to board_fdt_chosen_bootargs() to share the original 'chosen/bootargs' data with the board code. Consequently, the board developer can decide how to handle this information for their board setup: whether to drop it or merge it with the bootargs environment.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com --- boot/fdt_support.c | 5 +++-- include/fdt_support.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2c39b2dd14b7c447bc4c7c8fad64e22cb00e88e0..49efeec3681f7c97341ccdce596bcaa4e1f037eb 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -321,7 +321,7 @@ int fdt_kaslrseed(void *fdt, bool overwrite) * board_fdt_chosen_bootargs - boards may override this function to use * alternative kernel command line arguments */ -__weak const char *board_fdt_chosen_bootargs(void) +__weak const char *board_fdt_chosen_bootargs(const struct fdt_property *fdt_ba) { return env_get("bootargs"); } @@ -364,7 +364,8 @@ int fdt_chosen(void *fdt) } }
- str = board_fdt_chosen_bootargs(); + str = board_fdt_chosen_bootargs(fdt_get_property(fdt, nodeoffset, + "bootargs", NULL));
if (str) { err = fdt_setprop(fdt, nodeoffset, "bootargs", str, diff --git a/include/fdt_support.h b/include/fdt_support.h index f481881ce640797e0f6204d599f008d0e5dc2bf7..7731feba5f068f82c8ee25f18a9f0e57729e2bd8 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -231,12 +231,14 @@ int board_rng_seed(struct abuf *buf); /** * board_fdt_chosen_bootargs() - arbitrarily amend fdt kernel command line * + * @fdt_ba: FDT chosen/bootargs from the kernel image if available + * * This is used for late modification of kernel command line arguments just * before they are added into the /chosen node in flat device tree. * * Return: pointer to kernel command line arguments in memory */ -const char *board_fdt_chosen_bootargs(void); +const char *board_fdt_chosen_bootargs(const struct fdt_property *fdt_ba);
/* * The keystone2 SOC requires all 32 bit aliased addresses to be converted
Hi Dmitry,
On 12/19/24 10:42 PM, Dmitry Rokosov wrote:
Sometimes, it is necessary to provide an additional bootargs string to the kernel command line.
We have a real scenario where one U-Boot blob needs to boot several kernel images: the vendor-patched kernel image and the latest upstream kernel image. The Amlogic (Meson architecture) tty driver has different tty suffixes in these kernels: the vendor uses 'ttySx', while the upstream implementation uses 'ttyAMLx'. The initial console setup is provided to the kernel using the kernel command line (bootargs). For the vendor kernel, we should use 'console=ttyS0,115200', while for the upstream kernel, it must be 'console=ttyAML0,115200'. This means we have to use different command line strings depending on the kernel version.
Can't you have both in bootargs to handle both versions of the kernel with the same bootargs?
To resolve this issue, we cannot use the CMDLINE_EXTEND kernel configuration because it is considered legacy and is not supported for the arm64 architecture. CMDLINE_EXTEND is outdated primarily because we can provide additional command line strings through the 'chosen/bootargs' FDT node. However, U-Boot uses this node to inject the U-Boot bootargs environment variable content, which results in U-Boot silently overriding all data in the 'chosen/bootargs' node. While we do have the board_fdt_chosen_bootargs() board hook to address such issues, this function lacks any FDT context, such as the original value of the 'chosen/bootargs' node.
This patch introduces a read-only (RO) fdt_property argument to board_fdt_chosen_bootargs() to share the original 'chosen/bootargs' data with the board code. Consequently, the board developer can decide how to handle this information for their board setup: whether to drop it or merge it with the bootargs environment.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/fdt_support.c | 5 +++-- include/fdt_support.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2c39b2dd14b7c447bc4c7c8fad64e22cb00e88e0..49efeec3681f7c97341ccdce596bcaa4e1f037eb 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -321,7 +321,7 @@ int fdt_kaslrseed(void *fdt, bool overwrite)
- board_fdt_chosen_bootargs - boards may override this function to use
alternative kernel command line arguments*/ -__weak const char *board_fdt_chosen_bootargs(void) +__weak const char *board_fdt_chosen_bootargs(const struct fdt_property *fdt_ba) { return env_get("bootargs"); } @@ -364,7 +364,8 @@ int fdt_chosen(void *fdt) } }
- str = board_fdt_chosen_bootargs();
str = board_fdt_chosen_bootargs(fdt_get_property(fdt, nodeoffset,
"bootargs", NULL));if (str) { err = fdt_setprop(fdt, nodeoffset, "bootargs", str,
diff --git a/include/fdt_support.h b/include/fdt_support.h index f481881ce640797e0f6204d599f008d0e5dc2bf7..7731feba5f068f82c8ee25f18a9f0e57729e2bd8 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -231,12 +231,14 @@ int board_rng_seed(struct abuf *buf); /**
- board_fdt_chosen_bootargs() - arbitrarily amend fdt kernel command line
- @fdt_ba: FDT chosen/bootargs from the kernel image if available
Missing leading /
/chosen/bootargs
+property to highlight what this path refers to.
Makes sense to me otherwise!
With those changes, Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks! Quentin
Hello Quentin,
On Tue, Jan 14, 2025 at 12:16:35PM +0100, Quentin Schulz wrote:
Hi Dmitry,
On 12/19/24 10:42 PM, Dmitry Rokosov wrote:
Sometimes, it is necessary to provide an additional bootargs string to the kernel command line.
We have a real scenario where one U-Boot blob needs to boot several kernel images: the vendor-patched kernel image and the latest upstream kernel image. The Amlogic (Meson architecture) tty driver has different tty suffixes in these kernels: the vendor uses 'ttySx', while the upstream implementation uses 'ttyAMLx'. The initial console setup is provided to the kernel using the kernel command line (bootargs). For the vendor kernel, we should use 'console=ttyS0,115200', while for the upstream kernel, it must be 'console=ttyAML0,115200'. This means we have to use different command line strings depending on the kernel version.
Can't you have both in bootargs to handle both versions of the kernel with the same bootargs?
In this case, modifying the kernel source code is necessary, which falls outside the scope of this review.
To resolve this issue, we cannot use the CMDLINE_EXTEND kernel configuration because it is considered legacy and is not supported for the arm64 architecture. CMDLINE_EXTEND is outdated primarily because we can provide additional command line strings through the 'chosen/bootargs' FDT node. However, U-Boot uses this node to inject the U-Boot bootargs environment variable content, which results in U-Boot silently overriding all data in the 'chosen/bootargs' node. While we do have the board_fdt_chosen_bootargs() board hook to address such issues, this function lacks any FDT context, such as the original value of the 'chosen/bootargs' node.
This patch introduces a read-only (RO) fdt_property argument to board_fdt_chosen_bootargs() to share the original 'chosen/bootargs' data with the board code. Consequently, the board developer can decide how to handle this information for their board setup: whether to drop it or merge it with the bootargs environment.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/fdt_support.c | 5 +++-- include/fdt_support.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2c39b2dd14b7c447bc4c7c8fad64e22cb00e88e0..49efeec3681f7c97341ccdce596bcaa4e1f037eb 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -321,7 +321,7 @@ int fdt_kaslrseed(void *fdt, bool overwrite)
- board_fdt_chosen_bootargs - boards may override this function to use
alternative kernel command line arguments*/ -__weak const char *board_fdt_chosen_bootargs(void) +__weak const char *board_fdt_chosen_bootargs(const struct fdt_property *fdt_ba) { return env_get("bootargs"); } @@ -364,7 +364,8 @@ int fdt_chosen(void *fdt) } }
- str = board_fdt_chosen_bootargs();
- str = board_fdt_chosen_bootargs(fdt_get_property(fdt, nodeoffset,
"bootargs", NULL));diff --git a/include/fdt_support.h b/include/fdt_support.h index f481881ce640797e0f6204d599f008d0e5dc2bf7..7731feba5f068f82c8ee25f18a9f0e57729e2bd8 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -231,12 +231,14 @@ int board_rng_seed(struct abuf *buf); /**
- board_fdt_chosen_bootargs() - arbitrarily amend fdt kernel command line
- @fdt_ba: FDT chosen/bootargs from the kernel image if available
Missing leading /
/chosen/bootargs
+property to highlight what this path refers to.
Agree with that, but looks like patch series is already applied... Sorry..
Makes sense to me otherwise!
With those changes, Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks! Quentin
On Thu, Jan 16, 2025 at 04:11:40PM +0300, Dmitry Rokosov wrote:
Hello Quentin,
On Tue, Jan 14, 2025 at 12:16:35PM +0100, Quentin Schulz wrote:
Hi Dmitry,
On 12/19/24 10:42 PM, Dmitry Rokosov wrote:
Sometimes, it is necessary to provide an additional bootargs string to the kernel command line.
We have a real scenario where one U-Boot blob needs to boot several kernel images: the vendor-patched kernel image and the latest upstream kernel image. The Amlogic (Meson architecture) tty driver has different tty suffixes in these kernels: the vendor uses 'ttySx', while the upstream implementation uses 'ttyAMLx'. The initial console setup is provided to the kernel using the kernel command line (bootargs). For the vendor kernel, we should use 'console=ttyS0,115200', while for the upstream kernel, it must be 'console=ttyAML0,115200'. This means we have to use different command line strings depending on the kernel version.
Can't you have both in bootargs to handle both versions of the kernel with the same bootargs?
In this case, modifying the kernel source code is necessary, which falls outside the scope of this review.
To resolve this issue, we cannot use the CMDLINE_EXTEND kernel configuration because it is considered legacy and is not supported for the arm64 architecture. CMDLINE_EXTEND is outdated primarily because we can provide additional command line strings through the 'chosen/bootargs' FDT node. However, U-Boot uses this node to inject the U-Boot bootargs environment variable content, which results in U-Boot silently overriding all data in the 'chosen/bootargs' node. While we do have the board_fdt_chosen_bootargs() board hook to address such issues, this function lacks any FDT context, such as the original value of the 'chosen/bootargs' node.
This patch introduces a read-only (RO) fdt_property argument to board_fdt_chosen_bootargs() to share the original 'chosen/bootargs' data with the board code. Consequently, the board developer can decide how to handle this information for their board setup: whether to drop it or merge it with the bootargs environment.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/fdt_support.c | 5 +++-- include/fdt_support.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2c39b2dd14b7c447bc4c7c8fad64e22cb00e88e0..49efeec3681f7c97341ccdce596bcaa4e1f037eb 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -321,7 +321,7 @@ int fdt_kaslrseed(void *fdt, bool overwrite)
- board_fdt_chosen_bootargs - boards may override this function to use
alternative kernel command line arguments*/ -__weak const char *board_fdt_chosen_bootargs(void) +__weak const char *board_fdt_chosen_bootargs(const struct fdt_property *fdt_ba) { return env_get("bootargs"); } @@ -364,7 +364,8 @@ int fdt_chosen(void *fdt) } }
- str = board_fdt_chosen_bootargs();
- str = board_fdt_chosen_bootargs(fdt_get_property(fdt, nodeoffset,
"bootargs", NULL));diff --git a/include/fdt_support.h b/include/fdt_support.h index f481881ce640797e0f6204d599f008d0e5dc2bf7..7731feba5f068f82c8ee25f18a9f0e57729e2bd8 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -231,12 +231,14 @@ int board_rng_seed(struct abuf *buf); /**
- board_fdt_chosen_bootargs() - arbitrarily amend fdt kernel command line
- @fdt_ba: FDT chosen/bootargs from the kernel image if available
Missing leading /
/chosen/bootargs
+property to highlight what this path refers to.
Agree with that, but looks like patch series is already applied... Sorry..
I saw that and made those changes in the merge commit.
Hello Tom,
On Thu, Jan 16, 2025 at 01:11:25PM -0600, Tom Rini wrote:
On Thu, Jan 16, 2025 at 04:11:40PM +0300, Dmitry Rokosov wrote:
Hello Quentin,
On Tue, Jan 14, 2025 at 12:16:35PM +0100, Quentin Schulz wrote:
Hi Dmitry,
On 12/19/24 10:42 PM, Dmitry Rokosov wrote:
Sometimes, it is necessary to provide an additional bootargs string to the kernel command line.
We have a real scenario where one U-Boot blob needs to boot several kernel images: the vendor-patched kernel image and the latest upstream kernel image. The Amlogic (Meson architecture) tty driver has different tty suffixes in these kernels: the vendor uses 'ttySx', while the upstream implementation uses 'ttyAMLx'. The initial console setup is provided to the kernel using the kernel command line (bootargs). For the vendor kernel, we should use 'console=ttyS0,115200', while for the upstream kernel, it must be 'console=ttyAML0,115200'. This means we have to use different command line strings depending on the kernel version.
Can't you have both in bootargs to handle both versions of the kernel with the same bootargs?
In this case, modifying the kernel source code is necessary, which falls outside the scope of this review.
To resolve this issue, we cannot use the CMDLINE_EXTEND kernel configuration because it is considered legacy and is not supported for the arm64 architecture. CMDLINE_EXTEND is outdated primarily because we can provide additional command line strings through the 'chosen/bootargs' FDT node. However, U-Boot uses this node to inject the U-Boot bootargs environment variable content, which results in U-Boot silently overriding all data in the 'chosen/bootargs' node. While we do have the board_fdt_chosen_bootargs() board hook to address such issues, this function lacks any FDT context, such as the original value of the 'chosen/bootargs' node.
This patch introduces a read-only (RO) fdt_property argument to board_fdt_chosen_bootargs() to share the original 'chosen/bootargs' data with the board code. Consequently, the board developer can decide how to handle this information for their board setup: whether to drop it or merge it with the bootargs environment.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/fdt_support.c | 5 +++-- include/fdt_support.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2c39b2dd14b7c447bc4c7c8fad64e22cb00e88e0..49efeec3681f7c97341ccdce596bcaa4e1f037eb 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -321,7 +321,7 @@ int fdt_kaslrseed(void *fdt, bool overwrite)
- board_fdt_chosen_bootargs - boards may override this function to use
alternative kernel command line arguments*/ -__weak const char *board_fdt_chosen_bootargs(void) +__weak const char *board_fdt_chosen_bootargs(const struct fdt_property *fdt_ba) { return env_get("bootargs"); } @@ -364,7 +364,8 @@ int fdt_chosen(void *fdt) } }
- str = board_fdt_chosen_bootargs();
- str = board_fdt_chosen_bootargs(fdt_get_property(fdt, nodeoffset,
"bootargs", NULL));diff --git a/include/fdt_support.h b/include/fdt_support.h index f481881ce640797e0f6204d599f008d0e5dc2bf7..7731feba5f068f82c8ee25f18a9f0e57729e2bd8 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -231,12 +231,14 @@ int board_rng_seed(struct abuf *buf); /**
- board_fdt_chosen_bootargs() - arbitrarily amend fdt kernel command line
- @fdt_ba: FDT chosen/bootargs from the kernel image if available
Missing leading /
/chosen/bootargs
+property to highlight what this path refers to.
Agree with that, but looks like patch series is already applied... Sorry..
I saw that and made those changes in the merge commit.
Yep, I see the merge commit. Applied it to my uboot fork. Thanks a lot!
Hi Dmitry,
On Thu, 19 Dec 2024 at 14:42, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
Sometimes, it is necessary to provide an additional bootargs string to the kernel command line.
We have a real scenario where one U-Boot blob needs to boot several kernel images: the vendor-patched kernel image and the latest upstream kernel image. The Amlogic (Meson architecture) tty driver has different tty suffixes in these kernels: the vendor uses 'ttySx', while the upstream implementation uses 'ttyAMLx'. The initial console setup is provided to the kernel using the kernel command line (bootargs). For the vendor kernel, we should use 'console=ttyS0,115200', while for the upstream kernel, it must be 'console=ttyAML0,115200'. This means we have to use different command line strings depending on the kernel version.
To resolve this issue, we cannot use the CMDLINE_EXTEND kernel configuration because it is considered legacy and is not supported for the arm64 architecture. CMDLINE_EXTEND is outdated primarily because we can provide additional command line strings through the 'chosen/bootargs' FDT node. However, U-Boot uses this node to inject the U-Boot bootargs environment variable content, which results in U-Boot silently overriding all data in the 'chosen/bootargs' node. While we do have the board_fdt_chosen_bootargs() board hook to address such issues, this function lacks any FDT context, such as the original value of the 'chosen/bootargs' node.
This patch introduces a read-only (RO) fdt_property argument to board_fdt_chosen_bootargs() to share the original 'chosen/bootargs' data with the board code. Consequently, the board developer can decide how to handle this information for their board setup: whether to drop it or merge it with the bootargs environment.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/fdt_support.c | 5 +++-- include/fdt_support.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-)
You could look at 'bootflow cmdline' which allows changing individual parts of the bootargs. It uses library functions which can do insertion and deletion, etc.
Another longer-term point to make is that there is the concept of 'OS requests' in VBE. So long as the kernel is packaged in a FIT we could have an OS-Request property indicating the serial port that it needs.
Anyway, my comments are just some thoughts on how to solve this more generally.
Regards, Simon
Hello Simon,
On Tue, Jan 14, 2025 at 06:14:59AM -0700, Simon Glass wrote:
Hi Dmitry,
On Thu, 19 Dec 2024 at 14:42, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
Sometimes, it is necessary to provide an additional bootargs string to the kernel command line.
We have a real scenario where one U-Boot blob needs to boot several kernel images: the vendor-patched kernel image and the latest upstream kernel image. The Amlogic (Meson architecture) tty driver has different tty suffixes in these kernels: the vendor uses 'ttySx', while the upstream implementation uses 'ttyAMLx'. The initial console setup is provided to the kernel using the kernel command line (bootargs). For the vendor kernel, we should use 'console=ttyS0,115200', while for the upstream kernel, it must be 'console=ttyAML0,115200'. This means we have to use different command line strings depending on the kernel version.
To resolve this issue, we cannot use the CMDLINE_EXTEND kernel configuration because it is considered legacy and is not supported for the arm64 architecture. CMDLINE_EXTEND is outdated primarily because we can provide additional command line strings through the 'chosen/bootargs' FDT node. However, U-Boot uses this node to inject the U-Boot bootargs environment variable content, which results in U-Boot silently overriding all data in the 'chosen/bootargs' node. While we do have the board_fdt_chosen_bootargs() board hook to address such issues, this function lacks any FDT context, such as the original value of the 'chosen/bootargs' node.
This patch introduces a read-only (RO) fdt_property argument to board_fdt_chosen_bootargs() to share the original 'chosen/bootargs' data with the board code. Consequently, the board developer can decide how to handle this information for their board setup: whether to drop it or merge it with the bootargs environment.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/fdt_support.c | 5 +++-- include/fdt_support.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-)
You could look at 'bootflow cmdline' which allows changing individual parts of the bootargs. It uses library functions which can do insertion and deletion, etc.
Another longer-term point to make is that there is the concept of 'OS requests' in VBE. So long as the kernel is packaged in a FIT we could have an OS-Request property indicating the serial port that it needs.
Anyway, my comments are just some thoughts on how to solve this more generally.
Hmm, I wasn't aware of the OS-Request in VBE. I'll delve deeper into VBE and the 'bootflow cmdline' for our internal board bootflow cases. Thank you so much for the suggestion!
On Fri, 20 Dec 2024 00:42:07 +0300, Dmitry Rokosov wrote:
This series consists of three patches.
The first patch modifies the function documentation style in the include/fdt_support.h file to comply with kernel-doc requirements.
The second patch modifies the board_fdt_chosen_bootargs() function to return a const char* type. This change clarifies to the caller that the returned string should neither be freed nor modified. It aligns with the existing fdt_setprop() function, which already utilizes a const char* parameter. This promotes consistency within the codebase and enhances code safety by preventing unintended modifications to the returned string.
[...]
Applied to u-boot/master, thanks!
participants (5)
-
Dmitry Rokosov -
Heinrich Schuchardt -
Quentin Schulz -
Simon Glass -
Tom Rini