Hi Tom, On Tue, 7 Oct 2025 at 14:13, Tom Rini <[email protected]> wrote: > > On Tue, Oct 07, 2025 at 02:02:53PM -0600, Simon Glass wrote: > > Hi, > > > > On Tue, 7 Oct 2025 at 10:42, Tom Rini <[email protected]> wrote: > > > > > > On Tue, Oct 07, 2025 at 06:29:38PM +0200, Kory Maincent wrote: > > > > On Tue, 7 Oct 2025 08:11:48 -0600 > > > > Tom Rini <[email protected]> wrote: > > > > > > > > > On Tue, Oct 07, 2025 at 11:32:13AM +0200, Kory Maincent wrote: > > > > > > On Mon, 6 Oct 2025 17:42:40 -0600 > > > > > > Tom Rini <[email protected]> wrote: > > > > > > > > > > > > > On Mon, Oct 06, 2025 at 05:30:15PM -0600, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 6 Oct 2025 at 15:44, Tom Rini <[email protected]> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, Oct 06, 2025 at 02:38:23PM -0600, Simon Glass wrote: > > > > > > > > > > Hi Kory, > > > > > > > > > > > > > > > > > > > > On Fri, 3 Oct 2025 at 10:34, Kory Maincent > > > > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > > > > > > > Migrate TI board cape detection from legacy extension > > > > > > > > > > > support to > > > > > > > > > > > the new UCLASS-based extension board framework. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Kory Maincent <[email protected]> > > > > > > > > > > > > ... > > > > > > > > > > > > > > > > > -int extension_board_scan(struct list_head > > > > > > > > > > > *extension_list) > > > > > > > > > > > +static int ti_extension_board_scan(struct alist > > > > > > > > > > > *extension_list) > > > > > > > > > > > { > > > > > > > > > > > unsigned char addr; > > > > > > > > > > > int num_capes = 0; > > > > > > > > > > > @@ -31,7 +31,7 @@ int extension_board_scan(struct > > > > > > > > > > > list_head > > > > > > > > > > > *extension_list) struct am335x_cape_eeprom_id > > > > > > > > > > > eeprom_header; > > > > > > > > > > > char process_cape_part_number[17] = {'0'}; > > > > > > > > > > > char process_cape_version[5] = {'0'}; > > > > > > > > > > > - struct extension *cape; > > > > > > > > > > > + struct extension cape = {0}, *_cape; > > > > > > > > > > > struct udevice *dev; > > > > > > > > > > > u8 cursor = 0; > > > > > > > > > > > int ret, i; > > > > > > > > > > > @@ -78,22 +78,20 @@ int extension_board_scan(struct > > > > > > > > > > > list_head > > > > > > > > > > > *extension_list) > > > > > > > > > > > > > > > > > > > > I suppose that a method called scan() could be added to the > > > > > > > > > > uclass > > > > > > > > > > and implemented by this driver. > > > > > > > > > > > > I don't understand what do you mean by method? Is it different to > > > > > > the > > > > > > UBOOT driver scan ops I have designed? > > > > > > > > > > > > > > > > > printf("BeagleBone Cape: %s (0x%x)\n", > > > > > > > > > > > eeprom_header.board_name, addr); > > > > > > > > > > > > > > > > > > > > > > - cape = calloc(1, sizeof(struct > > > > > > > > > > > extension)); > > > > > > > > > > > - if (!cape) { > > > > > > > > > > > - printf("Error in memory > > > > > > > > > > > allocation\n"); > > > > > > > > > > > - return num_capes; > > > > > > > > > > > - } > > > > > > > > > > > - > > > > > > > > > > > - snprintf(cape->overlay, > > > > > > > > > > > sizeof(cape->overlay), > > > > > > > > > > > "%s-%s.dtbo", > > > > > > > > > > > + snprintf(cape.overlay, > > > > > > > > > > > sizeof(cape.overlay), > > > > > > > > > > > "%s-%s.dtbo", process_cape_part_number, > > > > > > > > > > > process_cape_version); > > > > > > > > > > > - strlcpy(cape->name, > > > > > > > > > > > eeprom_header.board_name, > > > > > > > > > > > + strlcpy(cape.name, > > > > > > > > > > > eeprom_header.board_name, > > > > > > > > > > > sizeof(eeprom_header.board_name)); > > > > > > > > > > > - strlcpy(cape->version, > > > > > > > > > > > process_cape_version, > > > > > > > > > > > + strlcpy(cape.version, > > > > > > > > > > > process_cape_version, > > > > > > > > > > > sizeof(process_cape_version)); > > > > > > > > > > > - strlcpy(cape->owner, > > > > > > > > > > > eeprom_header.manufacturer, > > > > > > > > > > > + strlcpy(cape.owner, > > > > > > > > > > > eeprom_header.manufacturer, > > > > > > > > > > > > > > > > > > > > > > sizeof(eeprom_header.manufacturer) + 1); > > > > > > > > > > > - list_add_tail(&cape->list, > > > > > > > > > > > extension_list); > > > > > > > > > > > + _cape = alist_add(extension_list, cape); > > > > > > > > > > > + if (!_cape) > > > > > > > > > > > + return -ENOMEM; > > > > > > > > > > > num_capes++; > > > > > > > > > > > } > > > > > > > > > > > return num_capes; > > > > > > > > > > > } > > > > > > > > > > > + > > > > > > > > > > > +U_BOOT_EXTENSION(cape, ti_extension_board_scan); > > > > > > > > > > > > > > > > > > > > Can the extension information go in the devicetree, so > > > > > > > > > > boards can > > > > > > > > > > select their scheme that way? > > > > > > > > > > > > > > > > > > Since we're scanning to see what extension boards are present > > > > > > > > > and so > > > > > > > > > what device tree overlays to apply, I don't think that can > > > > > > > > > work? > > > > > > > > > > > > > > > > My understanding is that the scanning process is > > > > > > > > board-specific. For > > > > > > > > example we would not want the Beagle mechanism to be compiled > > > > > > > > into > > > > > > > > rpi. The idea of scanning things in a board-specific way seems > > > > > > > > like a > > > > > > > > driver to me. Note there is also the SYSINFO uclass which can do > > > > > > > > similar things. > > > > > > > > > > > > > > > > The overlays being applied here are additions to what should > > > > > > > > already > > > > > > > > be a fairly complete devicetree. > > > > > > > > > > > > > > > > If this isn't possible, then where is the information known? > > > > > > > > > > > > Oh, so you would prefer to have something like the sysinfo drivers. > > > > > > > > > > > > > You're commenting on a patch to the TI specific code, that's only > > > > > > > used > > > > > > > by TI SoC using platforms that opt-in to the externally defined > > > > > > > EEPROM > > > > > > > format for these "capes". Perhaps your feedback would make more > > > > > > > sense in > > > > > > > some other part of the patch series? But even then, I'm not sure > > > > > > > it > > > > > > > makes sense? > > > > > > > > > > > > I am ok to move the cape scan detection code to a standalone driver > > > > > > with a > > > > > > devicetree compatible match. Just wondering what are the advantages > > > > > > to use > > > > > > standalone drivers instead of having the code in the specific board > > > > > > code as > > > > > > done in this series. > > > > > > > > > > I'm not sure there is an advantage. Especially since in theory we > > > > > could > > > > > have more than one built in, and if ti_extension_board_scan(...) > > > > > doesn't > > > > > find anything (because the EEPROM is missing/wrong) we could still > > > > > then > > > > > call rpi_extension_board_scan(...) and look for that formatted type of > > > > > EEPROM and then chipanddip_extension_board_scan(...) and look for that > > > > > and so forth. These extensions are nominally discoverable and so don't > > > > > need device tree compatibles, was some of the point of all this I > > > > > would > > > > > have sworn... > > > > > > > > I think, he means a driver for each type of custom extension board scan > > > > functions (4 until now). The U-boot devicetree would be like this: > > > > extension_scan { > > > > compatible = "bone-cape-scan"; > > > > }; > > > > > > > > We could avoid the build of several extension drivers using Kconfig > > > > choice > > > > symbol to select only one builtin. > > > > > > > > On my side I don't really have opinions, it is a matter of code design. > > > > For now > > > > I don't see real argument to tilt the balance except to do something > > > > like > > > > sysinfo. We need more arguments from Simon. > > > > > > Yes, and needing to go back to modifying the device trees seems a step > > > backwards here, and something that will be hard to argue for upstream. > > > > How do you define the mechanism to read the cape (etc.) information? > > Well, a cape for the TI platforms has to have a properly formatted > EEPROM. I forget how the CHIP/DIP thing worked, but I believe it was > similar (since it originated in the same era). I also believe Pi is > similar as well. > > > Presumably it is using a device which is already in the devicetree? So > > it is the actual extension mechanism that is not present, perhaps? > > > > How does Linux model extensions? > > Last I knew it doesn't and much discussion was had to arrive at that > point a decade or so ago. I believe FPGA stuff has been allowed since > then, but physical extensions not, and it's a bootloader problem. Which > is why we've been solving it for a while (it was more ad-hoc before Kory > did the current implementation in 2021).
OK, well the compatible string above at least allows a driver to be selected and all the board-specific stuff can go in there. Perhaps re can just reuse the sysinfo uclass? Perhaps we could talk about this in a future call? Regards, Simon
