driver/usbloader: rename RKUSBDriver to reflect rkdeveloptool usage

[a3f]

To interact with a Rockchip SoC in BootROM mode over USB, Rockchip
offers the rkdeveloptool, which is among others already packaged in
Debian.

We currently support this via the RKUSBDriver, which binds to the
RKUSBLoader resource, which currently lists a single VID/PID pair.

Actually making use of this is where things get confusing:
Labgrid will look up the rk-usb-loader key to find rkdeveloptool and then
fall back to a binary named rk-usb-loader. To my knowledge, no one names
their rkdeveloptool that way and rk-usb-loader has since become the
name of the Rockchip USB loader distributed by barebox.

On systems, like the LXA TAC, this is doubly confusing: There's a
rk-usb-loader binary, which Labgrid would use by default, but it's not
compatible with rkdeveloptool, supports only the RK35xx SoCs barebox
supports and is currently not supported by Labgrid at all.

Given that RKUSBLoader only supports a single SoC, our documentation is
wrong (it references the unrelated barebox' rk-usb-loader) and that
our usage of rkdeveloptool goes beyond the BootROM's USB protocol and
additionally flashes persistent media by talking to a first stage
usb_loader that's uploaded first, I think the best course of action
is to rename both the driver and the tool key to reflect that
rkdeveloptool is actually used.

---

I have yet to test the first (rename) commit, but posting it anyway to get some feedback if the rename is ok.

[a3f]

The support was originally added in #437. Pinging @otavio and @chtavares for their input.

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 56.5%. Comparing base (0304ec6) to head (203206a).

Files with missing lines	Patch %	Lines
labgrid/driver/usbloader.py	33.3%	2 Missing ⚠️
labgrid/remote/client.py	0.0%	1 Missing ⚠️
labgrid/resource/udev.py	0.0%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #1542   +/-   ##
======================================
  Coverage    56.5%   56.5%           
======================================
  Files         168     168           
  Lines       13111   13111           
======================================
  Hits         7417    7417           
  Misses       5694    5694           

Flag	Coverage Δ
3.10	56.5% <33.3%> (ø)
3.11	56.5% <33.3%> (ø)
3.12	56.5% <33.3%> (ø)
3.13	56.5% <33.3%> (ø)
3.9	56.6% <33.3%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[otavio]

Thanks for working on this and your changes are very good. I believe that this makes much clear for the user about the intended goal and the tool that is required for make it to work.

I also think that adding extra device support is very good.

[Emantor]

We usually let the old RKUSBDriver raise a DeprecationWarning and instead use the new driver, i.e. see NetworkUSBStorageDriver.

[a3f]

This won't work transparently, because I am changing the name of the tool key as well, but I can do this, so users are nudged into the right direction.

[QSchulz]

Sorry for chiming in with something that is not entirely related to those changes.

So we sell Rockchip-based SoMs and boards for the last 6-7 years and I've taken interest in labgrid but I've hit a few "weirdnesses" (to me) in that process, that may very well be because I misunderstood how labgrid works.

So, my setup is the following:

• executor on PC-A
• client on PC-B

RKUSBLoader is only matching devices with vendor id 0x2207 and model id 0x110a. I've no clue what that represents as the RK3399, PX30 and RK3588 I work with aren't matching that model id. For now I've just added mines in there but I was wondering if it would make more sense to have parameters for that Resource, à-la AndroidUSBFastboot for example. Otherwise we can simply add more to the matching list over time, not sure it's interesting to bind labgrid updates with Rockchip SoC support?

The tool to use to interact with the USB loader on Rockchip needs to run on the executor, but this needs to be specified on the Driver configuration side, which is part of the labgrid-device-config yaml file, which is on PC-B while it's going to run on PC-A. My issue is... there are currently three known tools for USB loading on Rockchip devices, rkdeveloptool, rockusb and this rk-usb-loader. I assume we could have some executors with one binary, and some others with another one. Why would the target conf file need to know on which executor something is going to run? Shouldn't this be related to a Place or the Resource itself?

[a3f]

RKUSBLoader is only matching devices with vendor id 0x2207 and model id 0x110a. I've no clue what that represents as the RK3399, PX30 and RK3588 I work with aren't matching that model id.

I am not sure either, maybe RV11?

For now I've just added mines in there but I was wondering if it would make more sense to have parameters for that Resource, à-la AndroidUSBFastboot for example.

I think it's useful anyhow to have the well-known VID/PIDs known to Labgrid. If it's possible to override them, it makes sense to have parameters for it, but the default should be that users add the VIDs/PIDs to Labgrid for shared benefit.

Otherwise we can simply add more to the matching list over time, not sure it's interesting to bind labgrid updates with Rockchip SoC support?

I don't find it unreasonable to update Labgrid when you are interfacing with a new SoC.

The tool to use to interact with the USB loader on Rockchip needs to run on the executor, but this needs to be specified on the Driver configuration side, which is part of the labgrid-device-config yaml file, which is on PC-B while it's going to run on PC-A. My issue is... there are currently three known tools for USB loading on Rockchip devices, rkdeveloptool, rockusb and this rk-usb-loader. I assume we could have some executors with one binary, and some others with another one. Why would the target conf file need to know on which executor something is going to run? Shouldn't this be related to a Place or the Resource itself?

I haven't used rockusb so far, but both rkdeveloptool and rk-usb-loader have an incompatile commnad-line interface.

For i.MX, there are multiple tools as well and Labgrid handles this by separating the resource (RKUSBLoader) from the Driver (which will be called RKDevelopToolUSBDriver in future). A future rk-usb-loader driver could then be called RKUSBLoaderDriver and each would implement the CLI of the underlying host tool.

Best case, both rk-usb-loader and rkdeveloptool are installed into $PATH and no one would need to care where exactly they are located, unless it needs to be overridden. This is already the case for other tools and the first patch in this PR has Rockchip USB support follow suit.

What the client needs to know about though is which driver should handle the resource. This isn't something that we can just leave to the exporter, because the tools tend to have their idiosyncrasies, which leak through the abstraction. Once the driver is configured in your environment YAML though (e.g. with the name of the USB loader binary to use), the rest of Labgrid can interface with it generically via the Boostrap protocol, e.g. in the client it's just labgrid-client -c myconfig.yaml bootstrap barebox-rk3399-pro-n10.img

[QSchulz]

RKUSBLoader is only matching devices with vendor id 0x2207 and model id 0x110a. I've no clue what that represents as the RK3399, PX30 and RK3588 I work with aren't matching that model id.

I am not sure either, maybe RV11?

RV1108 apparently :)

For now I've just added mines in there but I was wondering if it would make more sense to have parameters for that Resource, à-la AndroidUSBFastboot for example.

I think it's useful anyhow to have the well-known VID/PIDs known to Labgrid. If it's possible to override them, it makes sense to have parameters for it, but the default should be that users add the VIDs/PIDs to Labgrid for shared benefit.

Otherwise we can simply add more to the matching list over time, not sure it's interesting to bind labgrid updates with Rockchip SoC support?

I don't find it unreasonable to update Labgrid when you are interfacing with a new SoC.

I think it's unnecessary churn for the project and waiting time for people to be able to use labgrid with their yet-unsupported SoC. But that's a perfectly understandable policy, there's such policy for Yocto bbclasses for example, which is implemented in such a way it is very difficult to extend transparently.

The tool to use to interact with the USB loader on Rockchip needs to run on the executor, but this needs to be specified on the Driver configuration side, which is part of the labgrid-device-config yaml file, which is on PC-B while it's going to run on PC-A. My issue is... there are currently three known tools for USB loading on Rockchip devices, rkdeveloptool, rockusb and this rk-usb-loader. I assume we could have some executors with one binary, and some others with another one. Why would the target conf file need to know on which executor something is going to run? Shouldn't this be related to a Place or the Resource itself?

I haven't used rockusb so far, but both rkdeveloptool and rk-usb-loader have an incompatile commnad-line interface.

rockusb[1] implements a third one :)

rockusb --help
Usage: rockusb [OPTIONS] <COMMAND>

Commands:
  list
  download-boot
  read
  write
  write-file
  write-bmap
  chip-info
  flash-id
  flash-info
  reset-device
  nbd
  help           Print this message or the help of the given subcommand(s)

Options:
  -d, --device <DEVICE>  Device type specified as <bus>:<address>
  -h, --help             Print help

[1] https://github.com/collabora/rockchiprs/blob/main/rockusb/examples/rockusb.rs

though there is an issue by one of the contributors to add short parameters like rkdeveloptool[2]

[2] collabora/rockchiprs#17

For i.MX, there are multiple tools as well and Labgrid handles this by separating the resource (RKUSBLoader) from the Driver (which will be called RKDevelopToolUSBDriver in future). A future rk-usb-loader driver could then be called RKUSBLoaderDriver and each would implement the CLI of the underlying host tool.

I'm a bit worried by the rename because if we add support back for rk-usb-loader in the future, we'll have releases with "rk-usb-loader support but actually not really, the only way to use it is to point rk-usb-loader to rkdeveloptool with the tools: from the client config", other releases with "no rk-usb-loader support at all, rkdeveloptool is now through rkdeveloptoolusbdriver" and other releases with "actually now rk-usb-loader works, with the same driver as the releases that didn't work before". Which means we are breaking backward compatibility for the config file format (I don't know what's the stance of the project on that though).

[...]

What the client needs to know about though is which driver should handle the resource. This isn't something that we can just leave to the exporter, because the tools tend to have their idiosyncrasies, which leak through the abstraction. Once the driver is configured in your environment YAML though (e.g. with the name of the USB loader binary to use), the rest of Labgrid can interface with it generically via the Boostrap protocol, e.g. in the client it's just labgrid-client -c myconfig.yaml bootstrap barebox-rk3399-pro-n10.img

OK, so this means I need to have a huge myconfig.yaml, with all targets, that I maintain in a versioned repo that I could share with other devs I guess. It's a bit difficult for me to wrap my head around the potential sync issues between a place that is handled by the coordinator (and not versioned) and a client config file which could be defining a resource for a target via a place, those necessarily need to be in sync but by being different files, stored and used on different machines. Anyway, nothing related to this MR :) Thanks for the explanation.

[QSchulz]

suggestion (non-blocking): please comment on the SoC model those represents

This would make it easier to spot what's supported and what's not.

330c is RK3399
350b is RK3588

I also have:

330d for PX30 (but I can add myself, not an issue)

c.f. https://github.com/rockchip-linux/rkdeveloptool/blob/master/99-rk-rockusb.rules for some model ID

https://elixir.bootlin.com/u-boot/v2024.10/source/drivers/usb/gadget/Kconfig#L69 probably could help too :)

[obbardc]

I agree with this.

[a3f]

I didn't do that before, because I didn't know what the original VID/PID was for, but I can annotate them now. Thanks!

[a3f]

RKUSBLoader is only matching devices with vendor id 0x2207 and model id 0x110a. I've no clue what that represents as the RK3399, PX30 and RK3588 I work with aren't matching that model id.

I am not sure either, maybe RV11?

RV1108 apparently :)

Thanks for finding this out.

For now I've just added mines in there but I was wondering if it would make more sense to have parameters for that Resource, à-la AndroidUSBFastboot for example.

I think it's useful anyhow to have the well-known VID/PIDs known to Labgrid. If it's possible to override them, it makes sense to have parameters for it, but the default should be that users add the VIDs/PIDs to Labgrid for shared benefit.

Otherwise we can simply add more to the matching list over time, not sure it's interesting to bind labgrid updates with Rockchip SoC support?

I don't find it unreasonable to update Labgrid when you are interfacing with a new SoC.

I think it's unnecessary churn for the project and waiting time for people to be able to use labgrid with their yet-unsupported SoC.
But that's a perfectly understandable policy, there's such policy for Yocto bbclasses for example, which is implemented in such a way it is very difficult to extend transparently.

I'll have to defer to the maintainers what they think should be the policy.

The tool to use to interact with the USB loader on Rockchip needs to run on the executor, but this needs to be specified on the Driver configuration side, which is part of the labgrid-device-config yaml file, which is on PC-B while it's going to run on PC-A. My issue is... there are currently three known tools for USB loading on Rockchip devices, rkdeveloptool, rockusb and this rk-usb-loader. I assume we could have some executors with one binary, and some others with another one. Why would the target conf file need to know on which executor something is going to run? Shouldn't this be related to a Place or the Resource itself?

I haven't used rockusb so far, but both rkdeveloptool and rk-usb-loader have an incompatile commnad-line interface.

rockusb[1] implements a third one :)
[1] https://github.com/collabora/rockchiprs/blob/main/rockusb/examples/rockusb.rs

Heh, even more reason to rename the driver to reflect what is supports.

For i.MX, there are multiple tools as well and Labgrid handles this by separating the resource (RKUSBLoader) from the Driver (which will be called RKDevelopToolUSBDriver in future). A future rk-usb-loader driver could then be called RKUSBLoaderDriver and each would implement the CLI of the underlying host tool.

I'm a bit worried by the rename because if we add support back for rk-usb-loader in the future, we'll have releases with "rk-usb-loader support but actually not really, the only way to use it is to point rk-usb-loader to rkdeveloptool with the tools: from the client config",

Yes, this is confusing and this PR is rectifying the situation. I hope the impact will be limited as the driver seems not yet widely used evidenced by the single VID/PID match and the wrong documentation. I will add a deprecation warning and fallback as suggested by @Emantor.

other releases with "no rk-usb-loader support at all, rkdeveloptool is now through rkdeveloptoolusbdriver" and other releases with "actually now rk-usb-loader works, with the same driver as the releases that didn't work before". Which means we are breaking backward compatibility for the config file format (I don't know what's the stance of the project on that though).

Even in a future, where we have drivers for all three host utilities, the name RKUSBDriver will not be used again. But yes, a config file with:

    targets:
        drivers:
          RKUSBDriver:
            usb_loader: usb_loader
    tools:
      rk-usb-loader: /bin/rkdeveloptool

will cease to work, but we got to make a tradeoff between complicating things for everyone or breaking it for existing users. The authors of the change, which are the only users I knew of, are fine with it. I hope such breaking changes will be rare in future.

[...]

What the client needs to know about though is which driver should handle the resource. This isn't something that we can just leave to the exporter, because the tools tend to have their idiosyncrasies, which leak through the abstraction. Once the driver is configured in your environment YAML though (e.g. with the name of the USB loader binary to use), the rest of Labgrid can interface with it generically via the Boostrap protocol, e.g. in the client it's just labgrid-client -c myconfig.yaml bootstrap barebox-rk3399-pro-n10.img

OK, so this means I need to have a huge myconfig.yaml, with all targets, that I maintain in a versioned repo that I could share with other devs I guess. It's a bit difficult for me to wrap my head around the potential sync issues between a place that is handled by the coordinator (and not versioned) and a client config file which could be defining a resource for a target via a place, those necessarily need to be in sync but by being different files, stored and used on different machines. Anyway, nothing related to this MR :) Thanks for the explanation.

For test suites, you'll want to check in the environment yaml into the repository alongside the tests anyway. You can customize values with !template $LG_SOME_ENV_VAR if you don't want to hardcode something.

For interactive labgrid-client usage, the client already instantiates drivers by default to match against resources. You still need a config though for the USB loader.

Thinking about it, maybe we should change this in the same go: Other bootstrap drivers don't do persistent flashing of the bootstrapped image, maybe we should change the Rockchip USB driver to load only into RAM by default. That way labgrid-client usage may not need any USB loader binary and it will be possible config-less. Thoughts?

[QSchulz]

[...]

For i.MX, there are multiple tools as well and Labgrid handles this by separating the resource (RKUSBLoader) from the Driver (which will be called RKDevelopToolUSBDriver in future). A future rk-usb-loader driver could then be called RKUSBLoaderDriver and each would implement the CLI of the underlying host tool.

I'm a bit worried by the rename because if we add support back for rk-usb-loader in the future, we'll have releases with "rk-usb-loader support but actually not really, the only way to use it is to point rk-usb-loader to rkdeveloptool with the tools: from the client config",

Yes, this is confusing and this PR is rectifying the situation. I hope the impact will be limited as the driver seems not yet widely used evidenced by the single VID/PID match and the wrong documentation. I will add a deprecation warning and fallback as suggested by @Emantor.

other releases with "no rk-usb-loader support at all, rkdeveloptool is now through rkdeveloptoolusbdriver" and other releases with "actually now rk-usb-loader works, with the same driver as the releases that didn't work before". Which means we are breaking backward compatibility for the config file format (I don't know what's the stance of the project on that though).

Even in a future, where we have drivers for all three host utilities, the name RKUSBDriver will not be used again. But yes, a config file with:

RKUSBLoaderDriver was an awfully good name for a driver for rk-usb-loader though ;)

    targets:
        drivers:
          RKUSBDriver:
            usb_loader: usb_loader
    tools:
      rk-usb-loader: /bin/rkdeveloptool

I think we could simply add support for rkdeveloptool in another driver and keep RKUSBLoaderDriver (and maybe even fix its support?). While we won't support an rkdeveloptool in tools:rk-usb-loader anymore, it should be rather easy to warn users of that via doc update and parsing the config and warn/fail if rk-usb-loader containers rkdeveloptool and say "please use RKDevelopToolDriver instead" or something like that.

Also, I quickly went trhough the rk-usb-loader script and I think it is the equivalent of rkdeveloptool db <file> and that's it. So USB-loading a file.

Currently RKUSBLoaderDriver allows to flash an image after a special binary has been USB loaded through special commands (for rkdeveloptool, that is rkdeveloptool wl <block_offset> <file>. But I don't think we have an equivalent for rk-usb-loader. rockusb supports that though (via different parameters but still).

Also, I just remembered that rkdeveloptool wl (and other arguments) actually use the RKUSB protocol, of which not everything is supported in upstream U-Boot (or even in Rockchip blobs.....). I believe we also have the option of using DFU when USB loading upstream U-Boot, for which dfu-util should be used.

I'm therefore wondering if we shouldn't have clear separation of

• USB loading a binary
• interacting with the board which USB loaded the binary

We could very well use rk-usb-loader for the former and rockusb with the latter, or rkdeveloptool + df-util, etc...

will cease to work, but we got to make a tradeoff between complicating things for everyone or breaking it for existing users. The authors of the change, which are the only users I knew of, are fine with it. I hope such breaking changes will be rare in future.

kas is dealing with that with versioning the config format and warning/failing if you're using outdated config format with newer kas. Maybe something to investigate? In any case, if the breaking of backward compatibility is documented somewhere, that's fine to me :)

[...]

What the client needs to know about though is which driver should handle the resource. This isn't something that we can just leave to the exporter, because the tools tend to have their idiosyncrasies, which leak through the abstraction. Once the driver is configured in your environment YAML though (e.g. with the name of the USB loader binary to use), the rest of Labgrid can interface with it generically via the Boostrap protocol, e.g. in the client it's just labgrid-client -c myconfig.yaml bootstrap barebox-rk3399-pro-n10.img

OK, so this means I need to have a huge myconfig.yaml, with all targets, that I maintain in a versioned repo that I could share with other devs I guess. It's a bit difficult for me to wrap my head around the potential sync issues between a place that is handled by the coordinator (and not versioned) and a client config file which could be defining a resource for a target via a place, those necessarily need to be in sync but by being different files, stored and used on different machines. Anyway, nothing related to this MR :) Thanks for the explanation.

For test suites, you'll want to check in the environment yaml into the repository alongside the tests anyway. You can customize values with !template $LG_SOME_ENV_VAR if you don't want to hardcode something.

For interactive labgrid-client usage, the client already instantiates drivers by default to match against resources. You still need a config though for the USB loader.

Thanks, I'll need to finalize some kind of setup, make mistake and learn from them. A bit still too fresh in my head for now.

Thinking about it, maybe we should change this in the same go: Other bootstrap drivers don't do persistent flashing of the bootstrapped image, maybe we should change the Rockchip USB driver to load only into RAM by default. That way labgrid-client usage may not need any USB loader binary and it will be possible config-less. Thoughts?

AFAICT (haven't worked on it), on Rockchip we USB load TPL+SPL but then you have something very limited (proper + TF-A + OP-TEE + ... missing).

You have the option of doing rkusb (or rockusb (the protocol) whatever its actual name) via rkdeveloptool to do stuff (write somewhere for example) or using DFU (or probably also fastboot).

So, "USB loading" via rkdeveloptool db/rk-usb-loader doesn't actually bring your target to a CLI (maybe possible but not sure this is something one wants anyway). Is this what IMXUSBLoader is doing? I vaguely remember that you could USB boot a full bootloader via uuu (but that was ~10 years ago on an i.MX6 so /me shrugs).

[a3f]

Even in a future, where we have drivers for all three host utilities, the name RKUSBDriver will not be used again. But yes, a config file with:

RKUSBLoaderDriver was an awfully good name for a driver for rk-usb-loader though ;)

And this name wasn't used before. RKUSBLoader, which is the name of the resource remains as-is. Only RKUSBDriver is being changed.

I think we could simply add support for rkdeveloptool in another driver

That's what I am doing basically

and keep RKUSBLoaderDriver (and maybe even fix its support?).

I don't think adding support anew for rk-usb-loader can be considered a fix. But eventually, we may have a driver for it.

Note that the current name is RKUSBDriver, which is IMO a too generic name, which we should just never use again.

While we won't support an rkdeveloptool in tools:rk-usb-loader anymore, it should be rather easy to warn users of that via doc update and parsing the config and warn/fail if rk-usb-loader containers rkdeveloptool and say "please use RKDevelopToolDriver instead" or something like that.

I will look into extending the deprecation warning, so there are messages for both using the old driver name and the old tools key.

Also, I quickly went through the rk-usb-loader script and I think it is the equivalent of rkdeveloptool db <file> and that's it. So USB-loading a file.
Yes.

Currently RKUSBLoaderDriver allows to flash an image after a special binary has been USB loaded through special commands (for rkdeveloptool, that is rkdeveloptool wl <block_offset> <file>. But I don't think we have an equivalent for rk-usb-loader. rockusb supports that though (via different parameters but still).

I know. That's what the driver in Labgrid currently implements.

Also, I just remembered that rkdeveloptool wl (and other arguments) actually use the RKUSB protocol, of which not everything is supported in upstream U-Boot (or even in Rockchip blobs.....). I believe we also have the option of using DFU when USB loading upstream U-Boot, for which dfu-util should be used.

The current driver implements the BootstrapProtocol, whose hallmark is implementing a load function that loads a single file.

Fastboot and DFU go beyond that and they don't implement BootstrapProtocol. labgrid-client has dedicated commands for each, so the user has access to all functionality.

I'm therefore wondering if we shouldn't have clear separation of

* USB loading a binary

* interacting with the board which USB loaded the binary

Yes, I am starting to think a rename alone is insufficient. The proper abstraction would be a fastboot/DFU-esque driver that makes available the flashing, erasing, system reset functionality of the USB loader and BootstrapProtocol::load would just do the equivalent of rkdeveloptool db.

We could very well use rk-usb-loader for the former and rockusb with the latter, or rkdeveloptool + df-util, etc...

barebox only emulates SoC-specific USB gadget protocols if it's needed for the prebootloader to chainload barebox proper. The idea is that the tools in barebox scripts/ take a barebox binary and upload it, so it runs from RAM completely and you reach a shell and then Fastboot/DFU/Network boot whatever is used for persistent flashing. This flow is reflected in other BootstrapProtocols and only rkdeveloptool support seems to differ. So, yes, splitting it up is sensible.

will cease to work, but we got to make a tradeoff between complicating things for everyone or breaking it for existing users. The authors of the change, which are the only users I knew of, are fine with it. I hope such breaking changes will be rare in future.

kas is dealing with that with versioning the config format and warning/failing if you're using outdated config format with newer kas. Maybe something to investigate? In any case, if the breaking of backward compatibility is documented somewhere, that's fine to me :)

It's documented in the changelog and there will be deprecation warnings along with a fallback for a transitory period.

As a counterpoint to kas, docker has made their docker-compose.yml version field obsolete. I don't feel strongly either way, but there are tradeoffs involved and this would need more careful consideration.

Thinking about it, maybe we should change this in the same go: Other bootstrap drivers don't do persistent flashing of the bootstrapped image, maybe we should change the Rockchip USB driver to load only into RAM by default. That way labgrid-client usage may not need any USB loader binary and it will be possible config-less. Thoughts?

AFAICT (haven't worked on it), on Rockchip we USB load TPL+SPL but then you have something very limited (proper + TF-A + OP-TEE + ... missing).

rk-usb-loader only supports what barebox supports upstream, which at this time is RK3568 and RK3588. On both of these, it takes only the barebox image and the system boots to shell. I don't know if this is easily doable for older SoCs or this is just a side-effect of the new boot flow on the RK35xx and the RKNS format.

(The RK3399 I mention in the PR is not supported in barebox. Getting Labgrid to handle the RK3399 properly is part of my Yak shave to eventually upstream out-of-tree barebox RK3399/PX30 support).

You have the option of doing rkusb (or rockusb (the protocol) whatever its actual name) via rkdeveloptool to do stuff (write somewhere for example) or using DFU (or probably also fastboot).

Personally, I'd always go with Fastboot or DFU, so I am not too motivated to implement a driver for the rockusb protocol in full..

So, "USB loading" via rkdeveloptool db/rk-usb-loader doesn't actually bring your target to a CLI (maybe possible but not sure this is something one wants anyway).

That's how we use it. Then a script checks that "$bootsource" = "serial" and then barebox starts a composite gadget with fastboot/DFU and ACM in the background and then flashing is done via that.

Is this what IMXUSBLoader is doing? I vaguely remember that you could USB boot a full bootloader via uuu (but that was ~10 years ago on an i.MX6 so /me shrugs).

Yes, IMXUSBLoader boots to shell. uuu can talk both SDP/SDPS (BootROM protocol) and Fastboot with NXP vendor extensions (which of course doesn't use the OEM namespace). Personally, I stay clear of this and use the android-tools fastboot command for Fastboot stuff.

[QSchulz]

Even in a future, where we have drivers for all three host utilities, the name RKUSBDriver will not be used again. But yes, a config file with:

RKUSBLoaderDriver was an awfully good name for a driver for rk-usb-loader though ;)

And this name wasn't used before. RKUSBLoader, which is the name of the resource remains as-is. Only RKUSBDriver is being changed.

Well, if only I had taken the time to double check what I was saying... So I guess that's a +1 for the rename then since we'd probably have a driver named RKUSBLoaderDriver for rk-usb-loader (if anyone wants to implement that :) )?

I think we could simply add support for rkdeveloptool in another driver

That's what I am doing basically

and keep RKUSBLoaderDriver (and maybe even fix its support?).

I don't think adding support anew for rk-usb-loader can be considered a fix. But eventually, we may have a driver for it.

Note that the current name is RKUSBDriver, which is IMO a too generic name, which we should just never use again.

Agreed.

While we won't support an rkdeveloptool in tools:rk-usb-loader anymore, it should be rather easy to warn users of that via doc update and parsing the config and warn/fail if rk-usb-loader containers rkdeveloptool and say "please use RKDevelopToolDriver instead" or something like that.

I will look into extending the deprecation warning, so there are messages for both using the old driver name and the old tools key.

Also, I quickly went through the rk-usb-loader script and I think it is the equivalent of rkdeveloptool db <file> and that's it. So USB-loading a file.
Yes.

Currently RKUSBLoaderDriver allows to flash an image after a special binary has been USB loaded through special commands (for rkdeveloptool, that is rkdeveloptool wl <block_offset> <file>. But I don't think we have an equivalent for rk-usb-loader. rockusb supports that though (via different parameters but still).

I know. That's what the driver in Labgrid currently implements.

Also, I just remembered that rkdeveloptool wl (and other arguments) actually use the RKUSB protocol, of which not everything is supported in upstream U-Boot (or even in Rockchip blobs.....). I believe we also have the option of using DFU when USB loading upstream U-Boot, for which dfu-util should be used.

The current driver implements the BootstrapProtocol, whose hallmark is implementing a load function that loads a single file.

Fastboot and DFU go beyond that and they don't implement BootstrapProtocol. labgrid-client has dedicated commands for each, so the user has access to all functionality.

Thanks for the info. My point being that it depends what one means with 'bootstrapping'. If I'm not mistaken, U-Boot images that are USB loaded only contain TPL+SPL and then is waiting for proper via rockusb/rkusb protocol, fastboot or dfu, no CLI access. I believe it should be possible for fastboot and dfu to upload a file to DRAM and execute it, not sure for rockusb/rkusb though. I seem to recall Collabora is doing that very thing https://gitlab.collabora.com/hardware-enablement/rockchip-3588/notes-for-rockchip-3588/-/blob/main/upstream_uboot.md#how-to-create-a-blob-containing-ddr-init-and-spl-for-rockusb

So the issue here is that depending on the bootloader (I believe, haven't checked myself), we'd reach or not CLI with a simple rkdeveloptool db (Barebox vs U-Boot). This is.... kinda bad? Not sure how to handle that though.

I'm therefore wondering if we shouldn't have clear separation of

* USB loading a binary

* interacting with the board which USB loaded the binary

Yes, I am starting to think a rename alone is insufficient. The proper abstraction would be a fastboot/DFU-esque driver that makes available the flashing, erasing, system reset functionality of the USB loader and BootstrapProtocol::load would just do the equivalent of rkdeveloptool db.

FWIW, I would need to use rkdeveloptool with rockusb support for my board(s) for flashing/erasing/reset since USB gadget isn't working in upstream U-Boot (at the very least on PX30 Ringneck, RK3399 Puma and RK3588 Tiger may be fine), only with the blob, who only supports rockusb commands.

We could very well use rk-usb-loader for the former and rockusb with the latter, or rkdeveloptool + df-util, etc...

barebox only emulates SoC-specific USB gadget protocols if it's needed for the prebootloader to chainload barebox proper. The idea is that the tools in barebox scripts/ take a barebox binary and upload it, so it runs from RAM completely and you reach a shell and then Fastboot/DFU/Network boot whatever is used for persistent flashing. This flow is reflected in other BootstrapProtocols and only rkdeveloptool support seems to differ. So, yes, splitting it up is sensible.

OK. I am not sure this is possible in U-Boot SPL though. And if it is, I doubt it is common on Rockchip boards. I haven't had the need for USB loading upstream U-Boot so far, so have no experience with that.

will cease to work, but we got to make a tradeoff between complicating things for everyone or breaking it for existing users. The authors of the change, which are the only users I knew of, are fine with it. I hope such breaking changes will be rare in future.

kas is dealing with that with versioning the config format and warning/failing if you're using outdated config format with newer kas. Maybe something to investigate? In any case, if the breaking of backward compatibility is documented somewhere, that's fine to me :)

It's documented in the changelog and there will be deprecation warnings along with a fallback for a transitory period.

As a counterpoint to kas, docker has made their docker-compose.yml version field obsolete. I don't feel strongly either way, but there are tradeoffs involved and this would need more careful consideration.

Thanks for the info!

Thinking about it, maybe we should change this in the same go: Other bootstrap drivers don't do persistent flashing of the bootstrapped image, maybe we should change the Rockchip USB driver to load only into RAM by default. That way labgrid-client usage may not need any USB loader binary and it will be possible config-less. Thoughts?

AFAICT (haven't worked on it), on Rockchip we USB load TPL+SPL but then you have something very limited (proper + TF-A + OP-TEE + ... missing).

rk-usb-loader only supports what barebox supports upstream, which at this time is RK3568 and RK3588. On both of these, it takes only the barebox image and the system boots to shell. I don't know if this is easily doable for older SoCs or this is just a side-effect of the new boot flow on the RK35xx and the RKNS format.

Is there some documentation available somewhere for this new boot flow for USB aside from the code in Barebox? I was considering making U-Boot generate a USB bootable (TPL+SPL) image but the source code stopped being released a few years ago and no support for RK35xx in it.

(The RK3399 I mention in the PR is not supported in barebox. Getting Labgrid to handle the RK3399 properly is part of my Yak shave to eventually upstream out-of-tree barebox RK3399/PX30 support).

I have never built nor used Barebox so all I was and am saying is with respect to U-Boot.

You have the option of doing rkusb (or rockusb (the protocol) whatever its actual name) via rkdeveloptool to do stuff (write somewhere for example) or using DFU (or probably also fastboot).

Personally, I'd always go with Fastboot or DFU, so I am not too motivated to implement a driver for the rockusb protocol in full..

We have some support in upstream U-Boot (though if it is actually properly tested... who knows). FWIW, U-Boot currently doesn't generate a USB bootable image so I doubt many people are generating them by hand using Rockchip host blobs :) (boot_merger IIRC). I also have at least one board which doesn't have upstream support in U-Boot for USB gadget, so I still need to use Rockchip's blob until that is handled (not sure it's going to happen though as it also doesn't work in upstream Linux...).

So, "USB loading" via rkdeveloptool db/rk-usb-loader doesn't actually bring your target to a CLI (maybe possible but not sure this is something one wants anyway).

That's how we use it. Then a script checks that "$bootsource" = "serial" and then barebox starts a composite gadget with fastboot/DFU and ACM in the background and then flashing is done via that.

IIRC, in U-Boot we check if the boot source is USB and then start USB gadget with different functions (DFU/fastboot/rockusb)? But no CLI at that point, everything done in C code at build time.

[a3f]

So the issue here is that depending on the bootloader (I believe, haven't checked myself), we'd reach or not CLI with a simple rkdeveloptool db (Barebox vs U-Boot). This is.... kinda bad? Not sure how to handle that though.

Meh. I guess, we can just kick the can down the road by making usb_loader attribute optional. If it's set, it's used (rkdeveloptool wl), otherwise it's only rkdeveloptool db.

Yes, I am starting to think a rename alone is insufficient. The proper abstraction would be a fastboot/DFU-esque driver that makes available the flashing, erasing, system reset functionality of the USB loader and BootstrapProtocol::load would just do the equivalent of rkdeveloptool db.

FWIW, I would need to use rkdeveloptool with rockusb support for my board(s) for flashing/erasing/reset since USB gadget isn't working in upstream U-Boot (at the very least on PX30 Ringneck, RK3399 Puma and RK3588 Tiger may be fine), only with the blob, who only supports rockusb commands.

I see. Would you like to submit a follow-up PR bringing rockup support in Labgrid to parity with Fastboot? ;)

rk-usb-loader only supports what barebox supports upstream, which at this time is RK3568 and RK3588. On both of these, it takes only the barebox image and the system boots to shell. I don't know if this is easily doable for older SoCs or this is just a side-effect of the new boot flow on the RK35xx and the RKNS format.

Is there some documentation available somewhere for this new boot flow for USB aside from the code in Barebox?

I don't know of anything besides the code, sorry. I assume either BootROM or DDR blob has logic to continue USB boot into DRAM once RAM is setup.

I was considering making U-Boot generate a USB bootable (TPL+SPL) image but the source code stopped being released a few years ago and no support for RK35xx in it.

What does USB-bootable mean? The same barebox image placed on eMMC/SD/SPI can also be loaded via rk-usb-loader.

(The RK3399 I mention in the PR is not supported in barebox. Getting Labgrid to handle the RK3399 properly is part of my Yak shave to eventually upstream out-of-tree barebox RK3399/PX30 support).

I have never built nor used Barebox so all I was and am saying is with respect to U-Boot.

Never too late to start: https://barebox.org/demo ;)

You have the option of doing rkusb (or rockusb (the protocol) whatever its actual name) via rkdeveloptool to do stuff (write somewhere for example) or using DFU (or probably also fastboot).

Personally, I'd always go with Fastboot or DFU, so I am not too motivated to implement a driver for the rockusb protocol in full..

We have some support in upstream U-Boot (though if it is actually properly tested... who knows). FWIW, U-Boot currently doesn't generate a USB bootable image so I doubt many people are generating them by hand using Rockchip host blobs :) (boot_merger IIRC). I also have at least one board which doesn't have upstream support in U-Boot for USB gadget, so I still need to use Rockchip's blob until that is handled (not sure it's going to happen though as it also doesn't work in upstream Linux...).

I see.

So, "USB loading" via rkdeveloptool db/rk-usb-loader doesn't actually bring your target to a CLI (maybe possible but not sure this is something one wants anyway).

For me that's the point of bootstrap. Get a full bootloader running, so I can decide what to do next.

That's how we use it. Then a script checks that "$bootsource" = "serial" and then barebox starts a composite gadget with fastboot/DFU and ACM in the background and then flashing is done via that.

IIRC, in U-Boot we check if the boot source is USB and then start USB gadget with different functions (DFU/fastboot/rockusb)? But no CLI at that point, everything done in C code at build time.

I am grateful, I can use a single binary for everything.

[QSchulz]

So the issue here is that depending on the bootloader (I believe, haven't checked myself), we'd reach or not CLI with a simple rkdeveloptool db (Barebox vs U-Boot). This is.... kinda bad? Not sure how to handle that though.

Meh. I guess, we can just kick the can down the road by making usb_loader attribute optional. If it's set, it's used (rkdeveloptool wl), otherwise it's only rkdeveloptool db.

That would force us to use rkdeveloptool with the rockusb protocol, which for example doesn't implement erasing

Yes, I am starting to think a rename alone is insufficient. The proper abstraction would be a fastboot/DFU-esque driver that makes available the flashing, erasing, system reset functionality of the USB loader and BootstrapProtocol::load would just do the equivalent of rkdeveloptool db.

FWIW, I would need to use rkdeveloptool with rockusb support for my board(s) for flashing/erasing/reset since USB gadget isn't working in upstream U-Boot (at the very least on PX30 Ringneck, RK3399 Puma and RK3588 Tiger may be fine), only with the blob, who only supports rockusb commands.

I see. Would you like to submit a follow-up PR bringing rockup support in Labgrid to parity with Fastboot? ;)

What exactly are you suggesting here, I do not understand.

rk-usb-loader only supports what barebox supports upstream, which at this time is RK3568 and RK3588. On both of these, it takes only the barebox image and the system boots to shell. I don't know if this is easily doable for older SoCs or this is just a side-effect of the new boot flow on the RK35xx and the RKNS format.

Is there some documentation available somewhere for this new boot flow for USB aside from the code in Barebox?

I don't know of anything besides the code, sorry. I assume either BootROM or DDR blob has logic to continue USB boot into DRAM once RAM is setup.

I can explain how that works in U-Boot though :) The BootROM finds magic value which starts the expected header. It loads the DRAM init code in SRAM. The DRAM init stage (either through upstream U-Boot TPL with open-source DRAM init or via the blob provided by Rockchip) init the DRAM and then exits back to BootROM which continues reading from the medium according to info in the header and loads the next stage. In U-Boot, that's SPL. Then SPL loads from the same medium (or a fallback one) a fitImage which contains TF-A, OP-TEE and U-Boot proper. Loads TF-A, executes it. TF-A does its thing, changing the EL before executing into U-Boot proper. You'll notice that we don't do a back-to-bootrom step between SPL and TF-A.

I was considering making U-Boot generate a USB bootable (TPL+SPL) image but the source code stopped being released a few years ago and no support for RK35xx in it.

What does USB-bootable mean? The same barebox image placed on eMMC/SD/SPI can also be loaded via rk-usb-loader.

I need to look into Barebox source but I can already tell you that's not the case for U-Boot. We have one image for eMMC/SD and another for SPI. This is required for RK3399 (and older SoCs) because some SoCs have a broken BootROM implementation for the SPI controller which reads only the first half of every sector (what I have been told). I believe this is fine for SoCs with the headerv2 support in BootROM?

Also, we allow different location on storage medium depending on whether it's eMMC/SD or SPI (we use that on RK3399 Puma for example, which has a "small" SPI-NOR).

Considering Rockchip builds that USB loader (the one passed to rkdeveloptool db with boot_merger blob)... it seems odd that the same Barebox binary can be used on all kind of media. I shall investigate :) Thanks for the heads up!

You have the option of doing rkusb (or rockusb (the protocol) whatever its actual name) via rkdeveloptool to do stuff (write somewhere for example) or using DFU (or probably also fastboot).

Personally, I'd always go with Fastboot or DFU, so I am not too motivated to implement a driver for the rockusb protocol in full..

We have some support in upstream U-Boot (though if it is actually properly tested... who knows). FWIW, U-Boot currently doesn't generate a USB bootable image so I doubt many people are generating them by hand using Rockchip host blobs :) (boot_merger IIRC). I also have at least one board which doesn't have upstream support in U-Boot for USB gadget, so I still need to use Rockchip's blob until that is handled (not sure it's going to happen though as it also doesn't work in upstream Linux...).

I see.

So, "USB loading" via rkdeveloptool db/rk-usb-loader doesn't actually bring your target to a CLI (maybe possible but not sure this is something one wants anyway).

For me that's the point of bootstrap. Get a full bootloader running, so I can decide what to do next.

Got it.

So, to come clean. I'm currently investigating labgrid for automating U-Boot testing as this is quickly growing out of proportions with everything I need to test. One of the things I need to test is a fallback mechanism for U-Boot itself.

Basically, U-Boot TPL+SPL written to medium A, U-Boot proper written to medium B, U-Boot environment loaded from medium B. I have three storage media (not counting USB booting a whole U-Boot though that would be neat for manufacturing stage), so that makes a decent matrix of tests to run :)

I'm not asking you to support this use-case, just asking you to try to think about not making it impossible (I am not saying you're suggesting something that is making it impossible!)

I've derailed a bit the conversation from the original topic I believe though. Is there something I can help with/needs to be discussed wrt this PR?