-
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?
Changes from all commits
fc06fe2
6dfb873
291a7d7
8075580
8478e4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import pytest | ||
|
||
|
||
@pytest.fixture() | ||
def switchcfg(target): | ||
# bcu tool needs to be installed on workstation | ||
return target.get_driver('BCUResetDriver') | ||
|
||
@pytest.fixture() | ||
def shell(target): | ||
s = target.get_driver('ShellDriver') | ||
#in case board remained booted from a different test | ||
target.deactivate(s) | ||
return s | ||
|
||
@pytest.mark.lg_feature("SDcard") | ||
def test_boot_from_sdcard(target, switchcfg, shell): | ||
# Note that you need a bootable image already on sd | ||
switchcfg.sd() | ||
try: | ||
target.activate(shell) | ||
except: | ||
assert False, "Board failed to boot" | ||
stdout, stderr, returncode = shell.run('dmesg | grep root=/dev/mmcblk1') | ||
assert returncode == 0, "Board booted from wrong environment" | ||
|
||
def test_boot_from_emmc(target, switchcfg, shell): | ||
# Note that you need a bootable image already on emmc | ||
switchcfg.emmc() | ||
try: | ||
target.activate(shell) | ||
except: | ||
assert False, "Board failed to boot" | ||
stdout, stderr, returncode = shell.run('dmesg | grep root=/dev/mmcblk0') | ||
assert returncode == 0, "Board booted from wrong environment" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
features: | ||
- SDcard | ||
targets: | ||
main: | ||
resources: | ||
RawSerialPort: | ||
port: "/dev/ttyUSB2" | ||
USBResetPort: | ||
board: 'imx8dxlevk' | ||
match: | ||
'sys_name': '3-11.3' | ||
drivers: | ||
SerialDriver: {} | ||
BCUResetDriver: | ||
qspi_cfg: 'spi' | ||
ShellDriver: | ||
prompt: 'root@[\w-]+:[^ ]+ ' | ||
login_prompt: ' login: ' | ||
username: 'root' | ||
login_timeout: 180 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,12 @@ | |
import attr | ||
|
||
from ..factory import target_factory | ||
from ..protocol import DigitalOutputProtocol, ResetProtocol | ||
from ..protocol import DigitalOutputProtocol, ResetProtocol, BootCfgProtocol | ||
from ..resource.remote import NetworkUSBResetPort | ||
from ..resource.udev import USBResetPort | ||
from ..step import step | ||
from .common import Driver | ||
from ..util.helper import processwrapper | ||
|
||
@target_factory.reg_driver | ||
@attr.s(eq=False) | ||
|
@@ -24,3 +27,61 @@ def reset(self): | |
self.output.set(True) | ||
time.sleep(self.delay) | ||
self.output.set(False) | ||
|
||
@target_factory.reg_driver | ||
@attr.s(eq=False) | ||
class BCUResetDriver(Driver, ResetProtocol, BootCfgProtocol): | ||
"""BCUResetDriver - Driver using https://github.com/NXPmicro/bcu and board | ||
serial port as USBResetPort to reset the target into different states""" | ||
|
||
bindings = { | ||
"port": {USBResetPort, NetworkUSBResetPort}, | ||
} | ||
|
||
delay = attr.ib(default=1000, validator=attr.validators.instance_of(int)) | ||
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)) | ||
Comment on lines
+42
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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
How it looks in bcu:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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):
and then I have a state like this (state cycle reffer to power cycle of the board):
This means that for a board that has the configuration like this:
it will call bcu with bootmode emmc (the default config):
it will call bcu like this (using the config from yaml): And below is an example how strategy is used in test:
|
||
|
||
def __attrs_post_init__(self): | ||
super().__attrs_post_init__() | ||
self.tool = 'bcu' | ||
|
||
@Driver.check_active | ||
@step() | ||
def reset(self, mode=""): | ||
cmd = self.port.command_prefix + [ | ||
self.tool, | ||
"reset", mode, | ||
"-board={}".format(self.port.board), | ||
"-id={}".format(self.port.path), | ||
"-delay={}".format(str(self.delay)), | ||
] | ||
try: | ||
print(str(processwrapper.check_output(cmd).decode())) | ||
except subprocess.CalledProcessError as e: | ||
raise | ||
|
||
@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) | ||
Comment on lines
+65
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe 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). |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import abc | ||
|
||
|
||
class BootCfgProtocol(abc.ABC): | ||
@abc.abstractmethod | ||
def usb(self): | ||
raise NotImplementedError | ||
|
||
@abc.abstractmethod | ||
def sd(self): | ||
raise NotImplementedError | ||
|
||
@abc.abstractmethod | ||
def emmc(self): | ||
raise NotImplementedError | ||
|
||
@abc.abstractmethod | ||
def qspi(self): | ||
raise NotImplementedError | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -708,3 +708,15 @@ def filter_match(self, device): | |
return False | ||
|
||
return super().filter_match(device) | ||
|
||
@target_factory.reg_resource | ||
@attr.s(cmp=False) | ||
class USBResetPort(USBResource): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this name too generic? Perhaps 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 commentThe 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 |
||
"""The USBResetPort is the board serial cable (not the interface), | ||
it is identified via usb using udev and it accepts any vendor/product id | ||
|
||
Args: | ||
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 commentThe 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 commentThe 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 commentThe 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 commentThe 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 ? |
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
orNXPBCUDriver
. 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.