[RFC PATCH 0/2] board: ti: Use Extension framework
Hi,
RFC as only build tested. Need to get early feedback before I spend more time on this.
Newer TI boards (AM6/J7) are using a custom logic to detect expansion cards and set a 'name_overlays' environment variable. Then DT overlays are applied by again custom environment scripts during boot.
Instead of that migrate to using the Extension framework. This allows the user to call 'extension scan' and 'extension apply' commands to detect extension cards and apply the DT overlays respectively.
For Booting, I expect distro boot scripts to already support the extension framework via following environment 'extension_overlay_cmd' and 'extension_overlay_addr'. i.e. include/config_distro_bootcmd.h
cheers, -roger
Roger Quadros (2): board: ti: common: Add CONFIG_TI_CAPE_DETECT for cape detection board: ti: am65x: Move to using Extension framework
board/ti/am335x/board.c | 8 + board/ti/am65x/evm.c | 264 ++++++++--------------------- board/ti/common/Kconfig | 16 ++ board/ti/common/Makefile | 3 +- board/ti/common/cape_detect.c | 2 +- board/ti/common/cape_detect.h | 2 +- board/ti/common/ti_card_detect.c | 155 +++++++++++++++++ board/ti/common/ti_card_detect.h | 43 +++++ configs/am65x_evm_a53_defconfig | 2 + configs/am65x_hs_evm_a53_defconfig | 2 + 10 files changed, 299 insertions(+), 198 deletions(-) create mode 100644 board/ti/common/ti_card_detect.c create mode 100644 board/ti/common/ti_card_detect.h
CONFIG_CMD_EXTENSION may be used on other TI boards that may not require Cape detection logic.
Add CONFIG_TI_CAPE_DETECT for Cape detection logic so it can be enabled independently. Enable it by default only for platforms that require it. i.e. AM335X
Signed-off-by: Roger Quadros rogerq@kernel.org --- board/ti/am335x/board.c | 8 ++++++++ board/ti/common/Kconfig | 8 ++++++++ board/ti/common/Makefile | 2 +- board/ti/common/cape_detect.c | 2 +- board/ti/common/cape_detect.h | 2 +- 5 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c index ecb9fa02de..8bbabc5da8 100644 --- a/board/ti/am335x/board.c +++ b/board/ti/am335x/board.c @@ -86,6 +86,14 @@ void do_board_detect(void) } #endif
+int extension_board_scan(struct list_head *extension_list) +{ + if (IS_ENABLED(CONFIG_TI_CAPE_DETECT)) + return ti_cape_extension_board_scan(extension_list); + else + return -EINVAL; +} + #ifndef CONFIG_DM_SERIAL struct serial_device *default_serial_console(void) { diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig index 49edd98014..72ee2d6d0e 100644 --- a/board/ti/common/Kconfig +++ b/board/ti/common/Kconfig @@ -4,6 +4,14 @@ config TI_I2C_BOARD_DETECT Support for detection board information on Texas Instrument's Evaluation Boards which have I2C based EEPROM detection
+config TI_CAPE_DETECT + bool "Support BeagleBone Cape detection for TI platforms" + default y if TARGET_AM335X_EVM + depends on SUPPORT_EXTENSION_SCAN + help + Support Beagle Bone Cape detection for TI platforms + e.g. AM335x BeagleBone. + config EEPROM_BUS_ADDRESS int "Board EEPROM's I2C bus address" range 0 8 diff --git a/board/ti/common/Makefile b/board/ti/common/Makefile index 3172d87b46..5db433f77f 100644 --- a/board/ti/common/Makefile +++ b/board/ti/common/Makefile @@ -2,4 +2,4 @@ # Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
obj-${CONFIG_TI_I2C_BOARD_DETECT} += board_detect.o -obj-${CONFIG_CMD_EXTENSION} += cape_detect.o +obj-${CONFIG_TI_CAPE_DETECT} += cape_detect.o diff --git a/board/ti/common/cape_detect.c b/board/ti/common/cape_detect.c index 2e6105cfbf..d345a4d8eb 100644 --- a/board/ti/common/cape_detect.c +++ b/board/ti/common/cape_detect.c @@ -21,7 +21,7 @@ static void sanitize_field(char *text, size_t size) } }
-int extension_board_scan(struct list_head *extension_list) +int ti_cape_extension_board_scan(struct list_head *extension_list) { struct extension *cape; struct am335x_cape_eeprom_id eeprom_header; diff --git a/board/ti/common/cape_detect.h b/board/ti/common/cape_detect.h index b0d5c9f18b..ea486adbb2 100644 --- a/board/ti/common/cape_detect.h +++ b/board/ti/common/cape_detect.h @@ -23,6 +23,6 @@ struct am335x_cape_eeprom_id {
#define CAPE_MAGIC 0xEE3355AA
-int extension_board_scan(struct list_head *extension_list); +int ti_cape_extension_board_scan(struct list_head *extension_list);
#endif /* __CAPE_DETECT_H */
Support the Expansion cards via Extension framework. This should make 'expansion' command work to scan for expansion cards and apply DT overlays.
Card detection code is moved to a library so other boards can benefit from it.
Signed-off-by: Roger Quadros rogerq@kernel.org --- board/ti/am65x/evm.c | 264 ++++++++--------------------- board/ti/common/Kconfig | 8 + board/ti/common/Makefile | 1 + board/ti/common/ti_card_detect.c | 155 +++++++++++++++++ board/ti/common/ti_card_detect.h | 43 +++++ configs/am65x_evm_a53_defconfig | 2 + configs/am65x_hs_evm_a53_defconfig | 2 + 7 files changed, 280 insertions(+), 195 deletions(-) create mode 100644 board/ti/common/ti_card_detect.c create mode 100644 board/ti/common/ti_card_detect.h
diff --git a/board/ti/am65x/evm.c b/board/ti/am65x/evm.c index 706b219818..5bb187a062 100644 --- a/board/ti/am65x/evm.c +++ b/board/ti/am65x/evm.c @@ -22,21 +22,10 @@ #include <spl.h>
#include "../common/board_detect.h" +#include "../common/ti_card_detect.h"
#define board_is_am65x_base_board() board_ti_is("AM6-COMPROCEVM")
-/* Daughter card presence detection signals */ -enum { - AM65X_EVM_APP_BRD_DET, - AM65X_EVM_LCD_BRD_DET, - AM65X_EVM_SERDES_BRD_DET, - AM65X_EVM_HDMI_GPMC_BRD_DET, - AM65X_EVM_BRD_DET_COUNT, -}; - -/* Max number of MAC addresses that are parsed/processed per daughter card */ -#define DAUGHTER_CARD_NO_OF_MAC_ADDR 8 - /* Regiter that controls the SERDES0 lane and clock assignment */ #define CTRLMMR_SERDES0_CTRL 0x00104080 #define PCIE_LANE0 0x1 @@ -99,6 +88,74 @@ int board_fit_config_name_match(const char *name) } #endif
+/* Expansion card Slot IDs */ +enum { + AM65X_EVM_APP_BRD_DET, + AM65X_EVM_LCD_BRD_DET, + AM65X_EVM_SERDES_BRD_DET, + AM65X_EVM_HDMI_GPMC_BRD_DET, + AM65X_EVM_BRD_DET_COUNT, +}; + +/* Expansion card slots */ +static const struct ti_card_slot_map am65x_slot_map[] = { + { "gpio@38_0", 0x52, }, /* AM65X_EVM_APP_BRD_DET */ + { "gpio@38_1", 0x55, }, /* AM65X_EVM_LCD_BRD_DET */ + { "gpio@38_2", 0x54, }, /* AM65X_EVM_SERDES_BRD_DET */ + { "gpio@38_3", 0x53, }, /* AM65X_EVM_HDMI_GPMC_BRD_DET */ +}; + +/* Supported Expansion cards */ +static const struct ti_card_info am65x_card_info[] = { + { + AM65X_EVM_APP_BRD_DET, + "AM6-GPAPPEVM", + "k3-am654-gp.dtbo", + 0, + TI_CARD_OWNER, + TI_CARD_VERSION_1, + }, + { + AM65X_EVM_APP_BRD_DET, + "AM6-IDKAPPEVM", + "k3-am654-idk.dtbo", + 3, + TI_CARD_OWNER, + TI_CARD_VERSION_1, + }, + { + AM65X_EVM_SERDES_BRD_DET, + "SER-PCIE2LEVM", + "k3-am654-pcie-usb2.dtbo", + 0, + TI_CARD_OWNER, + TI_CARD_VERSION_1, + }, + { + AM65X_EVM_SERDES_BRD_DET, + "SER-PCIEUSBEVM", + "k3-am654-pcie-usb3.dtbo", + 0, + TI_CARD_OWNER, + TI_CARD_VERSION_1, + }, + { + AM65X_EVM_LCD_BRD_DET, + "OLDI-LCD1EVM", + "k3-am654-evm-oldi-lcd1evm.dtbo", + 0, + TI_CARD_OWNER, + TI_CARD_VERSION_1, + }, +}; + +int extension_board_scan(struct list_head *extension_list) +{ + return ti_card_detect(am65x_slot_map, ARRAY_SIZE(am65x_slot_map), + am65x_card_info, ARRAY_SIZE(am65x_card_info), + extension_list); +} + #ifdef CONFIG_TI_I2C_BOARD_DETECT int do_board_detect(void) { @@ -143,187 +200,7 @@ invalid_eeprom: set_board_info_env_am6(name); }
-static int init_daughtercard_det_gpio(char *gpio_name, struct gpio_desc *desc) -{ - int ret; - - memset(desc, 0, sizeof(*desc)); - - ret = dm_gpio_lookup_name(gpio_name, desc); - if (ret < 0) - return ret; - - /* Request GPIO, simply re-using the name as label */ - ret = dm_gpio_request(desc, gpio_name); - if (ret < 0) - return ret;
- return dm_gpio_set_dir_flags(desc, GPIOD_IS_IN); -} - -static int probe_daughtercards(void) -{ - struct ti_am6_eeprom ep; - struct gpio_desc board_det_gpios[AM65X_EVM_BRD_DET_COUNT]; - char mac_addr[DAUGHTER_CARD_NO_OF_MAC_ADDR][TI_EEPROM_HDR_ETH_ALEN]; - u8 mac_addr_cnt; - char name_overlays[1024] = { 0 }; - int i, j; - int ret; - - /* - * Daughter card presence detection signal name to GPIO (via I2C I/O - * expander @ address 0x38) name and EEPROM I2C address mapping. - */ - const struct { - char *gpio_name; - u8 i2c_addr; - } slot_map[AM65X_EVM_BRD_DET_COUNT] = { - { "gpio@38_0", 0x52, }, /* AM65X_EVM_APP_BRD_DET */ - { "gpio@38_1", 0x55, }, /* AM65X_EVM_LCD_BRD_DET */ - { "gpio@38_2", 0x54, }, /* AM65X_EVM_SERDES_BRD_DET */ - { "gpio@38_3", 0x53, }, /* AM65X_EVM_HDMI_GPMC_BRD_DET */ - }; - - /* Declaration of daughtercards to probe */ - const struct { - u8 slot_index; /* Slot the card is installed */ - char *card_name; /* EEPROM-programmed card name */ - char *dtbo_name; /* Device tree overlay to apply */ - u8 eth_offset; /* ethXaddr MAC address index offset */ - } cards[] = { - { - AM65X_EVM_APP_BRD_DET, - "AM6-GPAPPEVM", - "k3-am654-gp.dtbo", - 0, - }, - { - AM65X_EVM_APP_BRD_DET, - "AM6-IDKAPPEVM", - "k3-am654-idk.dtbo", - 3, - }, - { - AM65X_EVM_SERDES_BRD_DET, - "SER-PCIE2LEVM", - "k3-am654-pcie-usb2.dtbo", - 0, - }, - { - AM65X_EVM_SERDES_BRD_DET, - "SER-PCIEUSBEVM", - "k3-am654-pcie-usb3.dtbo", - 0, - }, - { - AM65X_EVM_LCD_BRD_DET, - "OLDI-LCD1EVM", - "k3-am654-evm-oldi-lcd1evm.dtbo", - 0, - }, - }; - - /* - * Initialize GPIO used for daughtercard slot presence detection and - * keep the resulting handles in local array for easier access. - */ - for (i = 0; i < AM65X_EVM_BRD_DET_COUNT; i++) { - ret = init_daughtercard_det_gpio(slot_map[i].gpio_name, - &board_det_gpios[i]); - if (ret < 0) - return ret; - } - - for (i = 0; i < ARRAY_SIZE(cards); i++) { - /* Obtain card-specific slot index and associated I2C address */ - u8 slot_index = cards[i].slot_index; - u8 i2c_addr = slot_map[slot_index].i2c_addr; - - /* - * The presence detection signal is active-low, hence skip - * over this card slot if anything other than 0 is returned. - */ - ret = dm_gpio_get_value(&board_det_gpios[slot_index]); - if (ret < 0) - return ret; - else if (ret) - continue; - - /* Get and parse the daughter card EEPROM record */ - ret = ti_i2c_eeprom_am6_get(CONFIG_EEPROM_BUS_ADDRESS, i2c_addr, - &ep, - (char **)mac_addr, - DAUGHTER_CARD_NO_OF_MAC_ADDR, - &mac_addr_cnt); - if (ret) { - pr_err("Reading daughtercard EEPROM at 0x%02x failed %d\n", - i2c_addr, ret); - /* - * Even this is pretty serious let's just skip over - * this particular daughtercard, rather than ending - * the probing process altogether. - */ - continue; - } - - /* Only process the parsed data if we found a match */ - if (strncmp(ep.name, cards[i].card_name, sizeof(ep.name))) - continue; - - printf("Detected: %s rev %s\n", ep.name, ep.version); - - /* - * Populate any MAC addresses from daughtercard into the U-Boot - * environment, starting with a card-specific offset so we can - * have multiple cards contribute to the MAC pool in a well- - * defined manner. - */ - for (j = 0; j < mac_addr_cnt; j++) { - if (!is_valid_ethaddr((u8 *)mac_addr[j])) - continue; - - eth_env_set_enetaddr_by_index("eth", - cards[i].eth_offset + j, - (uchar *)mac_addr[j]); - } - - /* - * It has been observed that setting SERDES0 lane mux to USB prevents USB - * 2.0 operation on USB0. Setting SERDES0 lane mux to non-USB when USB0 is - * used in USB 2.0 only mode solves this issue. For USB3.0+2.0 operation - * this issue is not present. - * - * Implement this workaround by writing 1 to LANE_FUNC_SEL field in - * CTRLMMR_SERDES0_CTRL register. - */ - if (!strncmp(ep.name, "SER-PCIE2LEVM", sizeof(ep.name))) - writel(PCIE_LANE0, CTRLMMR_SERDES0_CTRL); - - /* Skip if no overlays are to be added */ - if (!strlen(cards[i].dtbo_name)) - continue; - - /* - * Make sure we are not running out of buffer space by checking - * if we can fit the new overlay, a trailing space to be used - * as a separator, plus the terminating zero. - */ - if (strlen(name_overlays) + strlen(cards[i].dtbo_name) + 2 > - sizeof(name_overlays)) - return -ENOMEM; - - /* Append to our list of overlays */ - strcat(name_overlays, cards[i].dtbo_name); - strcat(name_overlays, " "); - } - - /* Apply device tree overlay(s) to the U-Boot environment, if any */ - if (strlen(name_overlays)) - return env_set("name_overlays", name_overlays); - - return 0; -} #endif
int board_late_init(void) @@ -340,9 +217,6 @@ int board_late_init(void) * an index of 1. */ board_ti_am6_set_ethaddr(1, ep->mac_addr_cnt); - - /* Check for and probe any plugged-in daughtercards */ - probe_daughtercards(); }
return 0; diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig index 72ee2d6d0e..039a2722f3 100644 --- a/board/ti/common/Kconfig +++ b/board/ti/common/Kconfig @@ -12,6 +12,14 @@ config TI_CAPE_DETECT Support Beagle Bone Cape detection for TI platforms e.g. AM335x BeagleBone.
+config TI_CARD_DETECT + bool "Support Expansion Card detection for TI platforms" + default y if TARGET_AM654_A53_EVM + depends on SUPPORT_EXTENSION_SCAN + help + Support Expansion Card detection for TI platforms + e.g. AM654-EVM + config EEPROM_BUS_ADDRESS int "Board EEPROM's I2C bus address" range 0 8 diff --git a/board/ti/common/Makefile b/board/ti/common/Makefile index 5db433f77f..f3922f231b 100644 --- a/board/ti/common/Makefile +++ b/board/ti/common/Makefile @@ -3,3 +3,4 @@
obj-${CONFIG_TI_I2C_BOARD_DETECT} += board_detect.o obj-${CONFIG_TI_CAPE_DETECT} += cape_detect.o +obj-${CONFIG_TI_CARD_DETECT} += ti_card_detect.o diff --git a/board/ti/common/ti_card_detect.c b/board/ti/common/ti_card_detect.c new file mode 100644 index 0000000000..7efb42cbbe --- /dev/null +++ b/board/ti/common/ti_card_detect.c @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * TI EVM Extension card handling + * + * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/ + */ + +#include <asm/gpio.h> +#include <asm/io.h> +#include <malloc.h> +#include <extension_board.h> +#include "board_detect.h" +#include "ti_card_detec.h" + +/* Max number of MAC addresses that are parsed/processed per daughter card */ +#define DAUGHTER_CARD_NO_OF_MAC_ADDR 8 + +static const char *k3_dtbo_list[AM64X_MAX_DAUGHTER_CARDS] = {NULL}; + +static int init_daughtercard_det_gpio(char *gpio_name, struct gpio_desc *desc) +{ + int ret; + + memset(desc, 0, sizeof(*desc)); + + ret = dm_gpio_lookup_name(gpio_name, desc); + if (ret < 0) + return ret; + + /* Request GPIO, simply re-using the name as label */ + ret = dm_gpio_request(desc, gpio_name); + if (ret < 0) + return ret; + + return dm_gpio_set_dir_flags(desc, GPIOD_IS_IN); +} + +int ti_card_detect(const struct ti_card_slot_map *slot_map, int num_slots, + const struct ti_card_info *cards, int num_cards, + struct list_head *extension_list) +{ + char mac_addr[DAUGHTER_CARD_NO_OF_MAC_ADDR][TI_EEPROM_HDR_ETH_ALEN]; + struct gpio_desc *board_det_gpios; + struct ti_am6_eeprom ep; + struct extension *extn; + u8 mac_addr_cnt; + int found_cards = 0; + int i, j; + int ret; + + board_det_gpios = calloc(num_slots, sizeof(struct gpio_desc)); + if (!board_det_gpios) + return -ENOMEM; + + /* + * Initialize GPIO used for daughtercard slot presence detection and + * keep the resulting handles in local array for easier access. + */ + for (i = 0; i < num_slots; i++) { + ret = init_daughtercard_det_gpio(slot_map[i].gpio_name, + &board_det_gpios[i]); + if (ret < 0) { + pr_err("Couldn't get Card detect GPIO for slot %d: %d\n", + i, ret); + return ret; + } + } + + for (i = 0; i < num_cards; i++) { + extn = calloc(1, sizeof(struct extension)); + /* freed in cmd/extension_board.c */ + if (!extn) { + pr_err("%s: Error in memory allocation\n", __func__); + return found_cards; + } + + /* Obtain card-specific slot index and associated I2C address */ + u8 slot_index = cards[i].slot_index; + u8 i2c_addr = slot_map[slot_index].i2c_addr; + + /* + * The presence detection signal is active-low, hence skip + * over this card slot if anything other than 0 is returned. + */ + ret = dm_gpio_get_value(&board_det_gpios[slot_index]); + if (ret < 0) + return ret; + else if (ret) + continue; + + /* Get and parse the daughter card EEPROM record */ + ret = ti_i2c_eeprom_am6_get(CONFIG_EEPROM_BUS_ADDRESS, i2c_addr, + &ep, + (char **)mac_addr, + DAUGHTER_CARD_NO_OF_MAC_ADDR, + &mac_addr_cnt); + if (ret) { + pr_err("Reading expansion card EEPROM at 0x%02x failed %d\n", + i2c_addr, ret); + /* + * Even this is pretty serious let's just skip over + * this particular daughtercard, rather than ending + * the probing process altogether. + */ + continue; + } + + /* Only process the parsed data if we found a match */ + if (strncmp(ep.name, cards[i].card_name, sizeof(ep.name))) + continue; + + printf("Detected: %s rev %s\n", ep.name, ep.version); + + /* + * Populate any MAC addresses from daughtercard into the U-Boot + * environment, starting with a card-specific offset so we can + * have multiple cards contribute to the MAC pool in a well- + * defined manner. + */ + for (j = 0; j < mac_addr_cnt; j++) { + if (!is_valid_ethaddr((u8 *)mac_addr[j])) + continue; + + eth_env_set_enetaddr_by_index("eth", + cards[i].eth_offset + j, + (uchar *)mac_addr[j]); + } + + /* + * It has been observed that setting SERDES0 lane mux to USB prevents USB + * 2.0 operation on USB0. Setting SERDES0 lane mux to non-USB when USB0 is + * used in USB 2.0 only mode solves this issue. For USB3.0+2.0 operation + * this issue is not present. + * + * Implement this workaround by writing 1 to LANE_FUNC_SEL field in + * CTRLMMR_SERDES0_CTRL register. + */ + if (!strncmp(ep.name, "SER-PCIE2LEVM", sizeof(ep.name))) + writel(PCIE_LANE0, CTRLMMR_SERDES0_CTRL); + + /* Skip if no overlays are to be added */ + if (!strlen(cards[i].dtbo_name)) + continue; + + strlcpy(extn->overlay, cards[i].dtbo_name, sizeof(extn->overlay)); + strlcpy(extn->name, cards[i].card_name, sizeof(extn->name)); + strlcpy(extn->version, cards[i].version, sizeof(extn->version)); + strlcpy(extn->owner, cards[i].owner, sizeof(extn->owner)); + list_add_tail(&extn->list, extension_list); + + found_cards++; + } + + return found_cards; +} diff --git a/board/ti/common/ti_card_detect.h b/board/ti/common/ti_card_detect.h new file mode 100644 index 0000000000..398376976a --- /dev/null +++ b/board/ti/common/ti_card_detect.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * TI EVM Extension card handling + * + * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/ + */ + +#define TI_CARD_OWNER "Texas Instruments Inc." +#define TI_CARD_VERSION_1 "v1" +/* + * ti_card_slot_map - A board may have 1 or more expansion slots. + * This data structure provides information for 1 slot. + * + * @det_gpio_name: GPIO name for the Expansion Card presense sense + * @card_i2c_addr: I2C address of expansion card EEPROM + */ +struct ti_card_slot_map { + char *det_gpio_name; + u8 card_i2c_addr; +}; + +/* + * ti_card_info - Information about a particular expansion card + * + * @slot_index: The Slot the card is typically installed in + * @card_name: EEPROM-programmed card name + * @dtbo_name: Device tree overlay for this card + * @eth_offset: ethXaddr MAC address index offset + * @owner: Manufacturer string + * @version: Version string + */ +struct ti_card_info { + u8 slot_index; + char *card_name; + char *dtbo_name; + u8 eth_offset; + char *owner; + char *version; +}; + +int ti_card_detect(const struct ti_card_slot_map *slot_map, int num_slots, + const struct ti_card_info *cards, int num_cards, + struct list_head *extension_list); diff --git a/configs/am65x_evm_a53_defconfig b/configs/am65x_evm_a53_defconfig index f294a4595f..e8e13479ce 100644 --- a/configs/am65x_evm_a53_defconfig +++ b/configs/am65x_evm_a53_defconfig @@ -179,3 +179,5 @@ CONFIG_USB_GADGET_VENDOR_NUM=0x0451 CONFIG_USB_GADGET_PRODUCT_NUM=0x6162 CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_PHANDLE_CHECK_SEQ=y +CONFIG_SUPPORT_EXTENSION_SCAN=y +CONFIG_CMD_EXTENSION=y diff --git a/configs/am65x_hs_evm_a53_defconfig b/configs/am65x_hs_evm_a53_defconfig index e0277d4787..ff569a9216 100644 --- a/configs/am65x_hs_evm_a53_defconfig +++ b/configs/am65x_hs_evm_a53_defconfig @@ -157,3 +157,5 @@ CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" CONFIG_USB_GADGET_VENDOR_NUM=0x0451 CONFIG_USB_GADGET_PRODUCT_NUM=0x6162 CONFIG_USB_GADGET_DOWNLOAD=y +CONFIG_SUPPORT_EXTENSION_SCAN=y +CONFIG_CMD_EXTENSION=y
Hi Roger,
On Mon, 10 Jul 2023 at 08:51, Roger Quadros rogerq@kernel.org wrote:
Support the Expansion cards via Extension framework. This should make 'expansion' command work to scan for expansion cards and apply DT overlays.
Card detection code is moved to a library so other boards can benefit from it.
Signed-off-by: Roger Quadros rogerq@kernel.org
board/ti/am65x/evm.c | 264 ++++++++--------------------- board/ti/common/Kconfig | 8 + board/ti/common/Makefile | 1 + board/ti/common/ti_card_detect.c | 155 +++++++++++++++++ board/ti/common/ti_card_detect.h | 43 +++++ configs/am65x_evm_a53_defconfig | 2 + configs/am65x_hs_evm_a53_defconfig | 2 + 7 files changed, 280 insertions(+), 195 deletions(-) create mode 100644 board/ti/common/ti_card_detect.c create mode 100644 board/ti/common/ti_card_detect.h
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
What do people think?
Regards, Simon
Hi Simon,
On 10/07/2023 22:45, Simon Glass wrote:
Hi Roger,
On Mon, 10 Jul 2023 at 08:51, Roger Quadros rogerq@kernel.org wrote:
Support the Expansion cards via Extension framework. This should make 'expansion' command work to scan for expansion cards and apply DT overlays.
Card detection code is moved to a library so other boards can benefit from it.
Signed-off-by: Roger Quadros rogerq@kernel.org
board/ti/am65x/evm.c | 264 ++++++++--------------------- board/ti/common/Kconfig | 8 + board/ti/common/Makefile | 1 + board/ti/common/ti_card_detect.c | 155 +++++++++++++++++ board/ti/common/ti_card_detect.h | 43 +++++ configs/am65x_evm_a53_defconfig | 2 + configs/am65x_hs_evm_a53_defconfig | 2 + 7 files changed, 280 insertions(+), 195 deletions(-) create mode 100644 board/ti/common/ti_card_detect.c create mode 100644 board/ti/common/ti_card_detect.h
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
ti_card_detect.c This is not a device driver but just a helper library which is ultimately going to be used directly by the board files.
e.g. see board/ti/am65x/evm.c
What do people think?
Regards, Simon
On 15:29-20230711, Roger Quadros wrote:
Hi Simon,
On 10/07/2023 22:45, Simon Glass wrote:
Hi Roger,
On Mon, 10 Jul 2023 at 08:51, Roger Quadros rogerq@kernel.org wrote:
Support the Expansion cards via Extension framework. This should make 'expansion' command work to scan for expansion cards and apply DT overlays.
Card detection code is moved to a library so other boards can benefit from it.
Signed-off-by: Roger Quadros rogerq@kernel.org
board/ti/am65x/evm.c | 264 ++++++++--------------------- board/ti/common/Kconfig | 8 + board/ti/common/Makefile | 1 + board/ti/common/ti_card_detect.c | 155 +++++++++++++++++ board/ti/common/ti_card_detect.h | 43 +++++ configs/am65x_evm_a53_defconfig | 2 + configs/am65x_hs_evm_a53_defconfig | 2 + 7 files changed, 280 insertions(+), 195 deletions(-) create mode 100644 board/ti/common/ti_card_detect.c create mode 100644 board/ti/common/ti_card_detect.h
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
ti_card_detect.c This is not a device driver but just a helper library which is ultimately going to be used directly by the board files.
e.g. see board/ti/am65x/evm.c
What do people think?
I think the linker list idea is better that library approach. One of the whack-a-mole we have been dealing with is with i2c eeprom detection board/ti/common/board_detect.c
The linker list idea could potentially allows us to have a common detect schemes with appropriate fall backs as pertinent to the platform (e.g. mid production update from 2byte to 1 byte eeprom etc) maybe the order indicated by defconfig? And adds capability to have fall through down to board type detection (thinking cape_detect / ti_card_detect etc..)..
Also boards that dont care for this can disable the configuration and use the rest of the codebase (simplify evm.c)
Just my 2 cents..
Hi,
On 13/07/2023 21:44, Nishanth Menon wrote:
On 15:29-20230711, Roger Quadros wrote:
Hi Simon,
On 10/07/2023 22:45, Simon Glass wrote:
Hi Roger,
On Mon, 10 Jul 2023 at 08:51, Roger Quadros rogerq@kernel.org wrote:
Support the Expansion cards via Extension framework. This should make 'expansion' command work to scan for expansion cards and apply DT overlays.
Card detection code is moved to a library so other boards can benefit from it.
Signed-off-by: Roger Quadros rogerq@kernel.org
board/ti/am65x/evm.c | 264 ++++++++--------------------- board/ti/common/Kconfig | 8 + board/ti/common/Makefile | 1 + board/ti/common/ti_card_detect.c | 155 +++++++++++++++++ board/ti/common/ti_card_detect.h | 43 +++++ configs/am65x_evm_a53_defconfig | 2 + configs/am65x_hs_evm_a53_defconfig | 2 + 7 files changed, 280 insertions(+), 195 deletions(-) create mode 100644 board/ti/common/ti_card_detect.c create mode 100644 board/ti/common/ti_card_detect.h
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
So I've been working on this on the side and got linker list way working with custom script booting but as soon as I move to standard boot flow it no longer works. This is because there is no code in place to apply the overlay and pass it to next stage e.g. EFI.
I see the following note at https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
" /* * TODO: Apply extension overlay * * Here we need to load and apply the extension overlay. This is * not implemented. See do_extension_apply(). The extension * stuff needs an implementation in boot/extension.c so it is * separate from the command code. Really the extension stuff * should use the device tree and a uclass / driver interface * rather than implementing its own list */ "
Another note at https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
"/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS uclass */"
So are we better off implementing a class driver for extension stuff?
Once that is in place how should extension apply work? Current implementation relies on a extension_overlay_cmd environment to be specified. e.g. for EFI boot case, the overlay files should be obtained in the same way we get the base device tree i.e. bootmeth_common_read_file()?
On Wed, 4 Oct 2023 15:39:23 +0300 Roger Quadros rogerq@kernel.org wrote:
Hello, Thanks for adding me in cc. Also it seems I forgot to add myself to MAINTAINERS for the extension_board.c file.
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
So I've been working on this on the side and got linker list way working with custom script booting but as soon as I move to standard boot flow it no longer works. This is because there is no code in place to apply the overlay and pass it to next stage e.g. EFI.
I see the following note at https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
" /* * TODO: Apply extension overlay * * Here we need to load and apply the extension overlay. This is * not implemented. See do_extension_apply(). The extension * stuff needs an implementation in boot/extension.c so it is * separate from the command code. Really the extension stuff * should use the device tree and a uclass / driver interface * rather than implementing its own list */ "
I agreed that the extension implementation should move in boot/extension.c or common for general use. I am wondering what the advantage of creating an uclass interface? I am not an uclass expert but there is no per driver ops and usage. What do you dislike about using its own list?
Another note at https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
"/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS uclass */"
So are we better off implementing a class driver for extension stuff?
I think the first point should be to move it in common or boot and makes it generic for using the extension function everywhere. I will let Simon answer about the uclass part.
Regards,
On 06/10/2023 16:26, Köry Maincent wrote:
On Wed, 4 Oct 2023 15:39:23 +0300 Roger Quadros rogerq@kernel.org wrote:
Hello, Thanks for adding me in cc. Also it seems I forgot to add myself to MAINTAINERS for the extension_board.c file.
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
So I've been working on this on the side and got linker list way working with custom script booting but as soon as I move to standard boot flow it no longer works. This is because there is no code in place to apply the overlay and pass it to next stage e.g. EFI.
I see the following note at https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
" /* * TODO: Apply extension overlay * * Here we need to load and apply the extension overlay. This is * not implemented. See do_extension_apply(). The extension * stuff needs an implementation in boot/extension.c so it is * separate from the command code. Really the extension stuff * should use the device tree and a uclass / driver interface * rather than implementing its own list */ "
I agreed that the extension implementation should move in boot/extension.c or common for general use. I am wondering what the advantage of creating an uclass interface? I am not an uclass expert but there is no per driver ops and usage. What do you dislike about using its own list?
There would be per platform ops for probing the extension cards in a platform specific way.
I already have a linker list way of doing this but stumbled upon Simon's comments about using uclass for this.
Another note at https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
"/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS uclass */"
So are we better off implementing a class driver for extension stuff?
I think the first point should be to move it in common or boot and makes it generic for using the extension function everywhere. I will let Simon answer about the uclass part.
Regards,
Hi Köry,
On Fri, 6 Oct 2023 at 07:26, Köry Maincent kory.maincent@bootlin.com wrote:
On Wed, 4 Oct 2023 15:39:23 +0300 Roger Quadros rogerq@kernel.org wrote:
Hello, Thanks for adding me in cc. Also it seems I forgot to add myself to MAINTAINERS for the extension_board.c file.
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
So I've been working on this on the side and got linker list way working with custom script booting but as soon as I move to standard boot flow it no longer works. This is because there is no code in place to apply the overlay and pass it to next stage e.g. EFI.
I see the following note at https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
" /* * TODO: Apply extension overlay * * Here we need to load and apply the extension overlay. This is * not implemented. See do_extension_apply(). The extension * stuff needs an implementation in boot/extension.c so it is * separate from the command code. Really the extension stuff * should use the device tree and a uclass / driver interface * rather than implementing its own list */ "
I agreed that the extension implementation should move in boot/extension.c or common for general use. I am wondering what the advantage of creating an uclass interface? I am not an uclass expert but there is no per driver ops and usage. What do you dislike about using its own list?
I looked at this briefly for bootstd and left it alone, partly for this reason.
The operations I know about are: - scan for extension boards to produce a list: needs to happen at start of 'bootflow scan' I think) - applying relevant overlays: needs to happen in the read_bootflow() method perhaps - listing available extensions
I think a uclass makes sense, along with separating out the 'extension' command from the actual functionality.
Another note at https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
"/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS uclass */"
So are we better off implementing a class driver for extension stuff?
I think the first point should be to move it in common or boot and makes it generic for using the extension function everywhere. I will let Simon answer about the uclass part.
I believe this relates to boot/
Regards,
Regards, Simon
Hi Köry,
On 06/10/2023 19:47, Simon Glass wrote:
Hi Köry,
On Fri, 6 Oct 2023 at 07:26, Köry Maincent kory.maincent@bootlin.com wrote:
On Wed, 4 Oct 2023 15:39:23 +0300 Roger Quadros rogerq@kernel.org wrote:
Hello, Thanks for adding me in cc. Also it seems I forgot to add myself to MAINTAINERS for the extension_board.c file.
Before this goes too far I think this should move to using a linker list to declare the driver (or a driver-model driver if you prefer, but that might be overkill).
So I've been working on this on the side and got linker list way working with custom script booting but as soon as I move to standard boot flow it no longer works. This is because there is no code in place to apply the overlay and pass it to next stage e.g. EFI.
I see the following note at https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
" /* * TODO: Apply extension overlay * * Here we need to load and apply the extension overlay. This is * not implemented. See do_extension_apply(). The extension * stuff needs an implementation in boot/extension.c so it is * separate from the command code. Really the extension stuff * should use the device tree and a uclass / driver interface * rather than implementing its own list */ "
I agreed that the extension implementation should move in boot/extension.c or common for general use. I am wondering what the advantage of creating an uclass interface? I am not an uclass expert but there is no per driver ops and usage. What do you dislike about using its own list?
I looked at this briefly for bootstd and left it alone, partly for this reason.
The operations I know about are:
- scan for extension boards to produce a list: needs to happen at
start of 'bootflow scan' I think)
- applying relevant overlays: needs to happen in the read_bootflow()
method perhaps
- listing available extensions
I think a uclass makes sense, along with separating out the 'extension' command from the actual functionality.
What do you think about this? OK to move extension card driver to uclass?
Another note at https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
"/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS uclass */"
So are we better off implementing a class driver for extension stuff?
I think the first point should be to move it in common or boot and makes it generic for using the extension function everywhere. I will let Simon answer about the uclass part.
I believe this relates to boot/
Regards,
Regards, Simon
participants (5)
-
Köry Maincent -
Nishanth Menon -
Roger Quadros -
Simon Glass -
Simon Glass