Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
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 wrote: >> >> On Wed, 4 Oct 2023 15:39:23 +0300 >> Roger Quadros 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 -- cheers, -roger
Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
Hi Köry, On Fri, 6 Oct 2023 at 07:26, Köry Maincent wrote: > > On Wed, 4 Oct 2023 15:39:23 +0300 > Roger Quadros 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
Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
On 06/10/2023 16:26, Köry Maincent wrote: > On Wed, 4 Oct 2023 15:39:23 +0300 > Roger Quadros 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, -- cheers, -roger
Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
On Wed, 4 Oct 2023 15:39:23 +0300 Roger Quadros 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,
Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
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 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 --- 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()? -- cheers, -roger
Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
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 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 > >> --- > >> 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.. -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
Hi Simon, On 10/07/2023 22:45, Simon Glass wrote: > Hi Roger, > > On Mon, 10 Jul 2023 at 08:51, Roger Quadros 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 >> --- >> 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 -- cheers, -roger
Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
Hi Roger, On Mon, 10 Jul 2023 at 08:51, Roger Quadros 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 > --- > 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
[RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework
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
---
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
#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_CTRL0x00104080
#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_
