Skip to content
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

driver: add BCUResetDriver to support bcu tool #1273

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ElenaGrigore
Copy link

@ElenaGrigore ElenaGrigore commented Sep 26, 2023

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:

  1. Connect one i.MX8DXL board to local workstation ; board already has plugged in an SDcard with a bootable image and also has a bootable image on the emmc ;
  2. Find out board serial using dmesg and unplug/plug serial cable ; third interface is the one for 8dxl main console - configured port /dev/ttyUSB2 in local.yaml (check example)
  3. Find out how to configure reset port using bcu lsftdi (3-11.3) and bcu lsboard (imx8dxlevk)
  4. Run example test:
    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

  • Documentation for the feature
  • Examples for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • PR has been tested
  • Man pages have been regenerated

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 <elena-mihaela.grigore@nxp.com>
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 <elena-mihaela.grigore@nxp.com>
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 <elena-mihaela.grigore@nxp.com>
Signed-off-by: Elena Grigore <elena-mihaela.grigore@nxp.com>
For this example to work you need to have bcu
tool installed and configured.

Signed-off-by: Elena Grigore <elena-mihaela.grigore@nxp.com>
@ElenaGrigore ElenaGrigore changed the title Bcudriver driver: add BCUResetDriver to support bcu tool Oct 6, 2023
@ElenaGrigore
Copy link
Author

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 ?

@Bastian-Krause
Copy link
Member

@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))
Copy link
Member

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.

Copy link
Author

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?

Copy link
Member

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?

Copy link
Author

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 ?

Comment on lines +65 to +86

@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)
Copy link
Member

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?

Copy link
Author

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 ?

Copy link
Member

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.

Copy link
Author

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.

Comment on lines +42 to +45
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))
Copy link
Member

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?

Copy link
Author

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 imx8ulpevk

available boot mode:

    fuse
    usb
    emmc_s
    emmc_nor_lp
    emmc_nor
    nand_nor
    nor_s
    nor_nor_lp
    nor_nor

done

Copy link
Member

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?

Copy link
Author

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):
Copy link
Member

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.

Copy link
Author

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):
Copy link
Member

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.

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants