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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 3944 bytes --]

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 ...

> 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.

Thanks,
-Takahiro Akashi


> 
> -- 
> Tom



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]