-
Notifications
You must be signed in to change notification settings - Fork 199
driver: add BCUResetDriver to support bcu tool #1273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Implement BootCfgProtocol that should be used by every driver that have the capability to change boot switches configuration from boards. Signed-off-by: Elena Grigore <[email protected]>
USBResetPort resource will be used by bcu tool to get the -id=<value> and -board=<value> - id is the path of the board serial can be retrieved with bcu lsftdi command - board is the board type and can be retrieved with bcu lsboard NetworkUSBResetPort is the remote resource for USBResetPort Signed-off-by: Elena Grigore <[email protected]>
BCUResetDriver is the driver that calls bcu tool. It binds to the USBResetPort (board serial) resource which should provide the id and the board. It runs the following command on the exporter: bcu reset <mode> -board=USBResetPort.board \ -id=USBResetPort.path \ -delay=BCUResetDriver.delay Signed-off-by: Elena Grigore <[email protected]>
Signed-off-by: Elena Grigore <[email protected]>
For this example to work you need to have bcu tool installed and configured. Signed-off-by: Elena Grigore <[email protected]>
Hi ! What is the flow for submitting a new driver ? I have tried to follow the steps from here: https://labgrid.readthedocs.io/en/latest/development.html#contributing . Is there anything else that I need to add/change ? |
@ElenaGrigore There's nothing missing from your side at the moment, we need to find the time to review this. |
board (str): mandatory arg to declare a board type recognized by bcu | ||
""" | ||
|
||
board = attr.ib(default=None, validator=attr.validators.instance_of(str)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the board be derived from USB VID/PID or any other info exposed via USB? That would avoid the need to configure it manually.
That way, the exporter would just need a list of USB paths and any compatible board could be used automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately no. This is the USB serial connection from the board and ftdi chip is the same on most of the boards you cannot detect the board type from it. But I can make it as a custom setting for the driver and not for the resource and in this way I wouldn't need a new resource created but just a resource of cls USBSerialPort. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All USBSerialPorts are exported via ser2net, which would probably conflict with local access to the device from the bcu tool. Adding RFC2217 protocol support to bcu is probably too much to ask. :)
Are you using the ttyUSB device or libftdi to communicate with the MCU on the board?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bcu is using libftdi as far as I know. In here better to keep the new port added but move the board attribute from resource and make it a driver attribute ? or leave it as it is ?
|
||
@Driver.check_active | ||
@step() | ||
def sd(self): | ||
self.reset(self.sd_cfg) | ||
|
||
@Driver.check_active | ||
@step() | ||
def emmc(self): | ||
self.reset(self.emmc_cfg) | ||
|
||
@Driver.check_active | ||
@step() | ||
def usb(self): | ||
self.reset(self.usb_cfg) | ||
|
||
@Driver.check_active | ||
@step() | ||
def qspi(self): | ||
if not self.qspi_cfg: | ||
print("QSPI mode is not set, board is reseting in current mode") | ||
self.reset(self.qspi_cfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we currently don't have other boot config drivers, I'd drop the BootCfgProtocol and just call reset("sd"
from the strategy or test cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the BootCfgProtocol because we have a second boot config driver which I was planning to submit and it helps in strategy - I call the same method from the protocol. Not sure how to address this for now ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. The supported boot modes will likely differ often from board to board, so hard-coding them in the BootCfgProtocol doesn't seem very useful. Perhaps only use a single method with a string option? Perhaps upload the code for your second driver somewhere (even if just in a https://gist.github.com/) so we could get a better impression.
The BootCfgProtocol class could still be added when adding the second implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so for bcu driver I will remove the BootCfgProtocol - indeed supported boot modes might differ between boards ; using a single method with string parameter seems a better option (I was thinking at it back then when I implemented the driver but I can't remember why I didn't use it).
I will try to upload the second driver as you suggested, but it might take some time.
emmc_cfg = attr.ib(default="emmc", validator=attr.validators.instance_of(str)) | ||
sd_cfg = attr.ib(default="sd", validator=attr.validators.instance_of(str)) | ||
usb_cfg = attr.ib(default="usb", validator=attr.validators.instance_of(str)) | ||
qspi_cfg = attr.ib(default="", validator=attr.validators.instance_of(str)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these different from board to board? How would custom settings look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly they are the same , but there are cases when they are different- usually qspi_cfg is different from board to board, and for the rest it might be the case where you need m_usb or m_emmc ... like from M core and not A core. Also you may have different emmc config for the same board and you need to select the right one: like for 8ULP
example config:
BCUResetDriver:
emmc_cfg: 'emmc_s'
How it looks in bcu:
sd@sd-01:~$ bcu lsbootmode -board=imx8ulpevk
version bcu_1.1.52-0-g17ab144
board model is imx8ulpevkavailable boot mode:
fuse usb emmc_s emmc_nor_lp emmc_nor nand_nor nor_s nor_nor_lp nor_nor
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, so you have 'emmc' as an alias which is used by the strategy and then maps to different boot modes on different places/boards?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in strategy I have a declaration like this (the controller part is related to the second driver that ca switch between boot modes , that one also can controll power):
def __attrs_post_init__(self):
super().__attrs_post_init__()
if "controller" in self.target.env.config.get_features():
self.switchcfg = self.power
if "bcu" in self.target.env.config.get_features():
self.switchcfg = self.target.get_driver(BCUResetDriver)
and then I have a state like this (state cycle reffer to power cycle of the board):
elif status == Status.emmc:
self.transition(Status.cycle)
try:
self.switchcfg.emmc()
except:
print("Board doesn't support remote switching or this state is not configured in board.yaml")
This means that for a board that has the configuration like this:
drivers:
BCUResetDriver:
qspi_cfg: 'spi'
it will call bcu with bootmode emmc (the default config): bcu reset emmc -board=.. -id=..
and for a board that has this configuration:
drivers:
BCUResetDriver:
emmc_cfg: 'emmc_s'
it will call bcu like this (using the config from yaml): bcu reset emmc_s -board=.. -id=..
And below is an example how strategy is used in test:
@pytest.mark.dependency(depends=['test_write_uboot_on_emmc'])
@pytest.mark.timeout(600)
@pytest.mark.flaky(reruns=2)
def test_boot_from_emmc_tftp_nfs(setup_teardown_test, target, uuu_strategy, request, tftp_nfs_config, uboot):
uuu_strategy.transition("emmc")
target.activate(uboot)
try:
uboot.run(tftp_nfs_config + " saveenv ;" + " run netboot")
except:
print("UbootDriver marker check failed or failed to stop at prompt ")
uuu_strategy.activate_shell_driver()
response = check_boot_patterns(request, target, patterns=[r"Boot:\s+("+BootModes.emmc.value+")"], exclude_board=["8M", "9"])
assert response, "Check Artifacts folder for full log"
|
||
@target_factory.reg_resource | ||
@attr.s(cmp=False) | ||
class USBResetPort(USBResource): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this name too generic? Perhaps BCUController
?
As BCU also supports power measurement, the same resource would also be used to export that functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, I will change the port name to something more specific: BCUController or BCUPort
|
||
@target_factory.reg_driver | ||
@attr.s(eq=False) | ||
class BCUResetDriver(Driver, ResetProtocol, BootCfgProtocol): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As BCU supports more than reset, perhaps this should be BCUDriver
or NXPBCUDriver
. And be in a separate file. Functions for power measurement could then be added to the driver later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, we just met recently a situation where another function from bcu (set_boot_mode) was needed.
Description
Implementation of BCUResetDriver which uses bcu tool: https://github.com/nxp-imx/bcu . This tool can be used on boards like i.MX8DXL or i.MX8MP in order to change the boot switches configuration: put the board in download mode, in sd mode, emmc mode an so on.
How this was tested locally:
pytest -s --verbose --tb=short --lg-log=examples/bcu --lg-env examples/bcu/local.yaml examples/bcu/bcu_example.py
Also, we use this change(driver) in our labgrid setup/farm and it is running in CI and for clients for 3 years - so multiple scenarios were tested. We have it using remote resources case. The example in this PR is for local usage.
Checklist