On Wed, Oct 08, 2025 at 06:54:09AM -0600, Simon Glass wrote: > 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?
I don't think we need to introduce more complexity here and Kory should just v2 the current design, given the comments he noted in the other half of the thread. It seems like some future kernel version may support this use case more clearly and then maybe there'll be some compatible string upstream to key off of. -- Tom
signature.asc
Description: PGP signature
