[PATCH v5 00/16] firmware: scmi: add SCMI base protocol support

[Tom Rini]

On Wed, Oct 11, 2023 at 10:36:11AM +0900, AKASHI Takahiro wrote:
> Hi Tom,
> 
> On Tue, Oct 10, 2023 at 10:19:09AM -0400, Tom Rini wrote:
> > On Tue, Sep 26, 2023 at 03:57:34PM +0900, AKASHI Takahiro wrote:
> > 
> > > This patch series allows users to access SCMI base protocol provided by
> > > SCMI server (platform). It will also be utilized in separate patches
> > > in the future to add sanity/validity checks for other protocols.
> > > See SCMI specification document v3.2 beta[1] for more details about SCMI
> > > base protocol.
> > > 
> > > What is currently not implemented is
> > > - SCMI_BASE_NOTIFY_ERRORS command and notification callback mechanism
> > > 
> > > This feature won't be very useful in the current U-Boot environment.
> > > 
> > > [1] https://developer.arm.com/documentation/den0056/e/?lang=en
> > > 
> > > 
> > > Test
> > > ====
> > > The patch series was tested on the following platforms:
> > > * sandbox
> > > * qemu-arm64 with OPTEE as SCMI server
> > 
> > I have two problems.  The first is that for sandbox, this series is not
> > bisectable as a commits fail to build due to needing a definition in the
> > test before it's added:
> > /home/uboot/work/u-boot/u-boot/test/dm/scmi.c: In function 'dm_test_scmi_clocks':
> > /home/uboot/work/u-boot/u-boot/test/dm/scmi.c:117:21: warning: implicit declaration of function 'scmi_get_protocol' [-Wimplicit-function-declaration]
> >   117 |         clock_dev = scmi_get_protocol(agent_dev, SCMI_PROTOCOL_ID_CLOCK);
> >       |                     ^~~~~~~~~~~~~~~~~
> 
> [snip]
> 
> Ah, I didn't notice this dependency.
> I will fix it by re-ordering the commits.
> 
> > Second, the series causes this failure:
> > ========================================== FAILURES ===========================================
> > _______________________________ test_ut[ut_dm_dm_test_scmi_cmd] _______________________________
> > test/py/u_boot_spawn.py:195: in expect
> >     c = os.read(self.fd, 1024).decode(errors='replace')
> > E   OSError: [Errno 5] Input/output error
> > 
> > During handling of the above exception, another exception occurred:
> > test/py/tests/test_ut.py:497: in test_ut
> >     output = u_boot_console.run_command('ut ' + ut_subtest)
> > test/py/u_boot_console_base.py:266: in run_command
> >     m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
> > test/py/u_boot_spawn.py:204: in expect
> >     raise ValueError('U-Boot exited with %s' % info)
> > E   ValueError: U-Boot exited with signal 11 (SIGSEGV)
> > ------------------------------------ Captured stdout call -------------------------------------
> > => ut dm dm_test_scmi_cmd
> > Test: dm_test_scmi_cmd: scmi.c
> > SCMI device: scmi
> >   protocol version: 0x20000
> >   # of agents: 2
> >       0: platform
> >     > 1: OSPM
> >   # of protocols: 3
> >       Clock management
> >       Reset domain management
> >       Voltage domain management
> >   vendor: U-Boot
> >   sub vendor: Sandbox
> >   impl version: 0x1
> > Denying access to device:0 failed (-13)
> > Denying access to protocol:0x14 on device:0 failed (-13)
> > Reset failed (-13)
> > Test: dm_test_scmi_cmd: scmi.c (flat tree)
> > SCMI device: ?n
> > =================================== short test summary info ===================================
> > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_scmi_cmd] - ValueError: U-Boot exited...
> 
> This seems to happen in the case of "flat tree", but
> I cannot reproduce it in my local environment with
>   $ pytest test/py/tests/test_ut.py -bd sandbox -k scmi_cmd ...

How about if you run all of the tests?  That's how I usually do it, and
CI as well.

> > Which I believe CI will also produce, once the issue with htmldocs is
> > fixed.  I can't bisect it exactly as the failure shows up once the new
> > test is added.
> 
> Anyhow, "scmi" command is more or less a debug tool for my development.
> All the functionality provided by this patch set is tested at "function"
> level with "ut dm scmi_base" and the command is *not* needed.
> To avoid increasing the maintenance cost, I will drop the command
> (patch #14-#16) in the next version.

OK.  Please make sure CI runs and passes as well, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231010/91a23e12/attachment.sig>