Re: [PATCH v5 14/16] cmd: add scmi command for SCMI firmware

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

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

Hi Tom, Michal,

On Tue, Oct 24, 2023 at 06:24:07PM -0400, Tom Rini wrote:
> On Tue, Oct 24, 2023 at 10:27:44AM +0200, Michal Simek wrote:
> > Hi Takahiro,
> > 
> > ?t 26. 9. 2023 v 9:00 odes?latel AKASHI Takahiro
> > <takahiro.akashi@linaro.org> napsal:
> > >
> > > This command, "scmi", may provide a command line interface to various SCMI
> > > protocols. It supports at least initially SCMI base protocol and is
> > > intended mainly for debug purpose.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
> > > ---
> > > v3
> > > * describe that arguments are in hex at a help message
> > > * modify the code for dynamically allocated agent names
> > > v2
> > > * remove sub command category, 'scmi base', for simplicity
> > > ---
> > >  cmd/Kconfig  |   9 ++
> > >  cmd/Makefile |   1 +
> > >  cmd/scmi.c   | 337 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 347 insertions(+)
> > >  create mode 100644 cmd/scmi.c
> > >
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index 43ca10f69ccf..f46152ace7d8 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -2533,6 +2533,15 @@ config CMD_CROS_EC
> > >           a number of sub-commands for performing EC tasks such as
> > >           updating its flash, accessing a small saved context area
> > >           and talking to the I2C bus behind the EC (if there is one).
> > > +
> > > +config CMD_SCMI
> > > +       bool "Enable scmi command"
> > > +       depends on SCMI_FIRMWARE
> > > +       default n
> > 
> > This line above is wrong and this was removed from v6 with

@Michal, do you mean the default should be 'y'?

> > "drop scmi command which was intended to be used for debugging".
> > It is fine that it shouldn't be used on production system but it
> > doesn't mean that
> > it should be actually removed.
> > It is useful for bring ups. Can we get this patch merged? It was
> > already reviewed
> > anyway.
> 
> There was then also some conflict with the follow-up series here. I
> would be fine with the command being introduced again after I merge that
> second series, which I am testing now.

I don't mind neither.
My concern, however, was a test, "ut dm scmi_cmd".
The output from "scmi info" command varies depending on what SCMI protocols
are provided by SCMI firmware (sandbox fake server in this case).

I will try to keep it updated.

-Takahiro Akashi

> 
> -- 
> Tom



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