diff mbox series

[09/10] doc: cmd: add documentation for scmi

Message ID 20230628004841.21774-10-takahiro.akashi@linaro.org
State New
Headers show
Series firmware: scmi: add SCMI base protocol support | expand

Commit Message

AKASHI Takahiro June 28, 2023, 12:48 a.m. UTC
This is a help text for scmi command.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 doc/usage/cmd/scmi.rst | 98 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100644 doc/usage/cmd/scmi.rst

Comments

Simon Glass June 29, 2023, 7:10 p.m. UTC | #1
Hi AKASHI,

On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> This is a help text for scmi command.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  doc/usage/cmd/scmi.rst | 98 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>  create mode 100644 doc/usage/cmd/scmi.rst
>
> diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst
> new file mode 100644
> index 000000000000..20cdae4b877d
> --- /dev/null
> +++ b/doc/usage/cmd/scmi.rst
> @@ -0,0 +1,98 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
> +
> +scmi command
> +============
> +
> +Synopsis
> +--------
> +
> +::
> +
> +    scmi base info
> +    scmi base perm_dev <agent id> <device id> <flags>
> +    scmi base perm_proto <agent id> <device id> <command id> <flags>
> +    scmi base reset <agent id> <flags>
> +
> +Description
> +-----------
> +
> +The scmi command is used to access and operate on SCMI server.
> +
> +scmi base info
> +~~~~~~~~~~~~~~
> +    Show base information about SCMI server and supported protocols
> +
> +scmi base perm_dev
> +~~~~~~~~~~~~~~~~~~
> +    Allow or deny access permission to the device
> +
> +scmi base perm_proto
> +~~~~~~~~~~~~~~~~~~~~
> +    Allow or deny access to the protocol on the device
> +
> +scmi base reset
> +~~~~~~~~~~~~~~~
> +    Reset the existing configurations
> +
> +Parameters are used as follows:
> +
> +<agent id>
> +    Agent ID

what is this?

> +
> +<device id>
> +    Device ID

what is this?

> +
> +<command id>
> +    Protocol ID, should not be 0x10 (base protocol)

what is this? Please add more detail

> +
> +<flags>
> +    Flags to control the action. See SCMI specification for
> +    defined values.

?

Please add the flags here, or at the very least provide a URL and page
number, etc.

> +
> +Example
> +-------
> +
> +Obtain basic information about SCMI server:
> +
> +::
> +
> +    => scmi base info
> +    SCMI device: scmi
> +      protocol version: 0x20000
> +      # of agents: 3
> +          0: platform
> +        > 1: OSPM
> +          2: PSCI
> +      # of protocols: 4
> +          Power domain management
> +          Performance domain management
> +          Clock management
> +          Sensor management
> +      vendor: Linaro
> +      sub vendor: PMWG
> +      impl version: 0x20b0000
> +
> +Ask for access permission to device#0:
> +
> +::
> +
> +    => scmi base perm_dev 1 0 1
> +
> +Reset configurations with all access permission settings retained:
> +
> +::
> +
> +    => scmi base reset 1 0
> +
> +Configuration
> +-------------
> +
> +The scmi command is only available if CONFIG_CMD_SCMI=y.
> +Default n because this command is mainly for debug purpose.
> +
> +Return value
> +------------
> +
> +The return value ($?) is set to 0 if the operation succeeded,
> +1 if the operation failed or -1 if the operation failed due to
> +a syntax error.
> --
> 2.41.0
>

Regards,
Simon
AKASHI Takahiro July 3, 2023, 1:19 a.m. UTC | #2
On Thu, Jun 29, 2023 at 08:10:02PM +0100, Simon Glass wrote:
> Hi AKASHI,
> 
> On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > This is a help text for scmi command.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  doc/usage/cmd/scmi.rst | 98 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >  create mode 100644 doc/usage/cmd/scmi.rst
> >
> > diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst
> > new file mode 100644
> > index 000000000000..20cdae4b877d
> > --- /dev/null
> > +++ b/doc/usage/cmd/scmi.rst
> > @@ -0,0 +1,98 @@
> > +.. SPDX-License-Identifier: GPL-2.0+:
> > +
> > +scmi command
> > +============
> > +
> > +Synopsis
> > +--------
> > +
> > +::
> > +
> > +    scmi base info
> > +    scmi base perm_dev <agent id> <device id> <flags>
> > +    scmi base perm_proto <agent id> <device id> <command id> <flags>
> > +    scmi base reset <agent id> <flags>
> > +
> > +Description
> > +-----------
> > +
> > +The scmi command is used to access and operate on SCMI server.
> > +
> > +scmi base info
> > +~~~~~~~~~~~~~~
> > +    Show base information about SCMI server and supported protocols
> > +
> > +scmi base perm_dev
> > +~~~~~~~~~~~~~~~~~~
> > +    Allow or deny access permission to the device
> > +
> > +scmi base perm_proto
> > +~~~~~~~~~~~~~~~~~~~~
> > +    Allow or deny access to the protocol on the device
> > +
> > +scmi base reset
> > +~~~~~~~~~~~~~~~
> > +    Reset the existing configurations
> > +
> > +Parameters are used as follows:
> > +
> > +<agent id>
> > +    Agent ID
> 
> what is this?

I thought that the meaning was trivial in SCMI context.
Will change it to "SCMI Agent ID".


> > +
> > +<device id>
> > +    Device ID
> 
> what is this?

Again, will change it to "SCMI Device ID".

> > +
> > +<command id>
> > +    Protocol ID, should not be 0x10 (base protocol)
> 
> what is this? Please add more detail

Again, will change it to "SCMI Protocol ID".
I think that users should be familiar with those terms
if they want to use these interfaces.

> > +
> > +<flags>
> > +    Flags to control the action. See SCMI specification for
> > +    defined values.
> 
> ?
> 
> Please add the flags here, or at the very least provide a URL and page
> number, etc.

I intentionally avoid providing details here because a set of flags
acceptable to a specific SCMI server may depend on the server
and its implementation version.
The interface on U-Boot is just a wrapper to make a call to SCMI server
via a transport layer and doesn't care what the parameters means.
That said, I agree to referring to a URL to SCMI specification somewhere
in this document.

Thanks,
-Takahiro Akashi

> > +
> > +Example
> > +-------
> > +
> > +Obtain basic information about SCMI server:
> > +
> > +::
> > +
> > +    => scmi base info
> > +    SCMI device: scmi
> > +      protocol version: 0x20000
> > +      # of agents: 3
> > +          0: platform
> > +        > 1: OSPM
> > +          2: PSCI
> > +      # of protocols: 4
> > +          Power domain management
> > +          Performance domain management
> > +          Clock management
> > +          Sensor management
> > +      vendor: Linaro
> > +      sub vendor: PMWG
> > +      impl version: 0x20b0000
> > +
> > +Ask for access permission to device#0:
> > +
> > +::
> > +
> > +    => scmi base perm_dev 1 0 1
> > +
> > +Reset configurations with all access permission settings retained:
> > +
> > +::
> > +
> > +    => scmi base reset 1 0
> > +
> > +Configuration
> > +-------------
> > +
> > +The scmi command is only available if CONFIG_CMD_SCMI=y.
> > +Default n because this command is mainly for debug purpose.
> > +
> > +Return value
> > +------------
> > +
> > +The return value ($?) is set to 0 if the operation succeeded,
> > +1 if the operation failed or -1 if the operation failed due to
> > +a syntax error.
> > --
> > 2.41.0
> >
> 
> Regards,
> Simon
Simon Glass July 3, 2023, 1:30 p.m. UTC | #3
Hi ,

On Mon, 3 Jul 2023 at 02:19, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> On Thu, Jun 29, 2023 at 08:10:02PM +0100, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > This is a help text for scmi command.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > >  doc/usage/cmd/scmi.rst | 98 ++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 98 insertions(+)
> > >  create mode 100644 doc/usage/cmd/scmi.rst
> > >
> > > diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst
> > > new file mode 100644
> > > index 000000000000..20cdae4b877d
> > > --- /dev/null
> > > +++ b/doc/usage/cmd/scmi.rst
> > > @@ -0,0 +1,98 @@
> > > +.. SPDX-License-Identifier: GPL-2.0+:
> > > +
> > > +scmi command
> > > +============
> > > +
> > > +Synopsis
> > > +--------
> > > +
> > > +::
> > > +
> > > +    scmi base info
> > > +    scmi base perm_dev <agent id> <device id> <flags>
> > > +    scmi base perm_proto <agent id> <device id> <command id> <flags>
> > > +    scmi base reset <agent id> <flags>
> > > +
> > > +Description
> > > +-----------
> > > +
> > > +The scmi command is used to access and operate on SCMI server.
> > > +
> > > +scmi base info
> > > +~~~~~~~~~~~~~~
> > > +    Show base information about SCMI server and supported protocols
> > > +
> > > +scmi base perm_dev
> > > +~~~~~~~~~~~~~~~~~~
> > > +    Allow or deny access permission to the device
> > > +
> > > +scmi base perm_proto
> > > +~~~~~~~~~~~~~~~~~~~~
> > > +    Allow or deny access to the protocol on the device
> > > +
> > > +scmi base reset
> > > +~~~~~~~~~~~~~~~
> > > +    Reset the existing configurations
> > > +
> > > +Parameters are used as follows:
> > > +
> > > +<agent id>
> > > +    Agent ID
> >
> > what is this?
>
> I thought that the meaning was trivial in SCMI context.
> Will change it to "SCMI Agent ID".

What is SCMI? Really you should explain and link to information about
things you mention in documentation. How else is anyone supposed to
figure out what you are talking about?

"SCMI Agent ID" doesn't tell us much. What form does it take? Is it a
string? How do you find it out?

>
>
> > > +
> > > +<device id>
> > > +    Device ID
> >
> > what is this?
>
> Again, will change it to "SCMI Device ID".

That doesn't help much. The purpose of documentation is to explain
things for people who don't already know it. We need to try to be as
helpful as possible.

>
> > > +
> > > +<command id>
> > > +    Protocol ID, should not be 0x10 (base protocol)
> >
> > what is this? Please add more detail
>
> Again, will change it to "SCMI Protocol ID".
> I think that users should be familiar with those terms
> if they want to use these interfaces.

Maybe, but it still isn't clear what it is. A string? It is confusing
that the command ID is also a protocol ID.

>
> > > +
> > > +<flags>
> > > +    Flags to control the action. See SCMI specification for
> > > +    defined values.
> >
> > ?
> >
> > Please add the flags here, or at the very least provide a URL and page
> > number, etc.
>
> I intentionally avoid providing details here because a set of flags
> acceptable to a specific SCMI server may depend on the server
> and its implementation version.
> The interface on U-Boot is just a wrapper to make a call to SCMI server
> via a transport layer and doesn't care what the parameters means.
> That said, I agree to referring to a URL to SCMI specification somewhere
> in this document.

That will help. But also please add that this is a hex value (right?)


>
> Thanks,
> -Takahiro Akashi
>
> > > +
> > > +Example
> > > +-------
> > > +
> > > +Obtain basic information about SCMI server:
> > > +
> > > +::
> > > +
> > > +    => scmi base info
> > > +    SCMI device: scmi
> > > +      protocol version: 0x20000
> > > +      # of agents: 3
> > > +          0: platform
> > > +        > 1: OSPM
> > > +          2: PSCI
> > > +      # of protocols: 4
> > > +          Power domain management
> > > +          Performance domain management
> > > +          Clock management
> > > +          Sensor management
> > > +      vendor: Linaro
> > > +      sub vendor: PMWG
> > > +      impl version: 0x20b0000
> > > +
> > > +Ask for access permission to device#0:
> > > +
> > > +::
> > > +
> > > +    => scmi base perm_dev 1 0 1
> > > +
> > > +Reset configurations with all access permission settings retained:
> > > +
> > > +::
> > > +
> > > +    => scmi base reset 1 0
> > > +
> > > +Configuration
> > > +-------------
> > > +
> > > +The scmi command is only available if CONFIG_CMD_SCMI=y.
> > > +Default n because this command is mainly for debug purpose.
> > > +
> > > +Return value
> > > +------------
> > > +
> > > +The return value ($?) is set to 0 if the operation succeeded,
> > > +1 if the operation failed or -1 if the operation failed due to
> > > +a syntax error.
> > > --
> > > 2.41.0
> > >
Regards,
Simon
AKASHI Takahiro July 4, 2023, 2:05 a.m. UTC | #4
Hi Simon,

On Mon, Jul 03, 2023 at 02:30:55PM +0100, Simon Glass wrote:
> Hi ,
> 
> On Mon, 3 Jul 2023 at 02:19, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > On Thu, Jun 29, 2023 at 08:10:02PM +0100, Simon Glass wrote:
> > > Hi AKASHI,
> > >
> > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > <takahiro.akashi@linaro.org> wrote:
> > > >
> > > > This is a help text for scmi command.
> > > >
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > >  doc/usage/cmd/scmi.rst | 98 ++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 98 insertions(+)
> > > >  create mode 100644 doc/usage/cmd/scmi.rst
> > > >
> > > > diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst
> > > > new file mode 100644
> > > > index 000000000000..20cdae4b877d
> > > > --- /dev/null
> > > > +++ b/doc/usage/cmd/scmi.rst
> > > > @@ -0,0 +1,98 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0+:
> > > > +
> > > > +scmi command
> > > > +============
> > > > +
> > > > +Synopsis
> > > > +--------
> > > > +
> > > > +::
> > > > +
> > > > +    scmi base info
> > > > +    scmi base perm_dev <agent id> <device id> <flags>
> > > > +    scmi base perm_proto <agent id> <device id> <command id> <flags>
> > > > +    scmi base reset <agent id> <flags>
> > > > +
> > > > +Description
> > > > +-----------
> > > > +
> > > > +The scmi command is used to access and operate on SCMI server.
> > > > +
> > > > +scmi base info
> > > > +~~~~~~~~~~~~~~
> > > > +    Show base information about SCMI server and supported protocols
> > > > +
> > > > +scmi base perm_dev
> > > > +~~~~~~~~~~~~~~~~~~
> > > > +    Allow or deny access permission to the device
> > > > +
> > > > +scmi base perm_proto
> > > > +~~~~~~~~~~~~~~~~~~~~
> > > > +    Allow or deny access to the protocol on the device
> > > > +
> > > > +scmi base reset
> > > > +~~~~~~~~~~~~~~~
> > > > +    Reset the existing configurations
> > > > +
> > > > +Parameters are used as follows:
> > > > +
> > > > +<agent id>
> > > > +    Agent ID
> > >
> > > what is this?
> >
> > I thought that the meaning was trivial in SCMI context.
> > Will change it to "SCMI Agent ID".
> 
> What is SCMI? Really you should explain and link to information about
> things you mention in documentation. How else is anyone supposed to

Generally yes, but please note that SCMI and related drivers are already
there in U-Boot. For instance, see drivers/firmware/scmi/Kconfig.
I don't think we need any more explanation about what is SCMI everywhere
the word, "SCMI", appears.

> figure out what you are talking about?

Again, those who don't know about SCMI and if SCMI server is installed
on their systems will never use this command.

> 
> "SCMI Agent ID" doesn't tell us much. What form does it take? Is it a
> string? How do you find it out?

While I still believe that What SCMI agent ID means is trivial for
those who read the SCMI specification, I will give a short description
about possible values.

> >
> >
> > > > +
> > > > +<device id>
> > > > +    Device ID
> > >
> > > what is this?
> >
> > Again, will change it to "SCMI Device ID".
> 
> That doesn't help much. The purpose of documentation is to explain
> things for people who don't already know it. We need to try to be as
> helpful as possible.

The same above.
Please note that the definition of "device (ID)", except its data type,
is out of scope of SCMI specification.
It doesn't describe any means by which values be identified/retrieved.

> >
> > > > +
> > > > +<command id>
> > > > +    Protocol ID, should not be 0x10 (base protocol)
> > >
> > > what is this? Please add more detail
> >
> > Again, will change it to "SCMI Protocol ID".
> > I think that users should be familiar with those terms
> > if they want to use these interfaces.
> 
> Maybe, but it still isn't clear what it is. A string?

Will add a short description about its data type/format.

> It is confusing
> that the command ID is also a protocol ID.

Yes, I know, but the confusion exists in SCMI specification.
It sometimes seems to use both words almost interchangeably, even
in a single context. For instance, the section 4.2.2.11 of [1]
says,

4.2.2.11 BASE_SET_PROTOCOL_PERMISSIONS
 ...
 This command BASE_SET_PROTOCOL_PERMISSIONS is used to ...
 (This "command" has a yet another meaning.)

Parameters
uint32_t command_id     Bits[31:8] Reserved, must be zero
                        Bits[7:0] Protocol ID

[1]  https://developer.arm.com/documentation/den0056/e/?lang=en

So using "command_id" for a parameter name and "Protocol ID"
as a (part of) value is very much aligned with the specification.

That said, I will change "command_id" to "protocol_id" for now.

> >
> > > > +
> > > > +<flags>
> > > > +    Flags to control the action. See SCMI specification for
> > > > +    defined values.
> > >
> > > ?
> > >
> > > Please add the flags here, or at the very least provide a URL and page
> > > number, etc.
> >
> > I intentionally avoid providing details here because a set of flags
> > acceptable to a specific SCMI server may depend on the server
> > and its implementation version.
> > The interface on U-Boot is just a wrapper to make a call to SCMI server
> > via a transport layer and doesn't care what the parameters means.
> > That said, I agree to referring to a URL to SCMI specification somewhere
> > in this document.
> 
> That will help. But also please add that this is a hex value (right?)

Will do.

Thanks,
-Takahiro Akashi

> 
> >
> > Thanks,
> > -Takahiro Akashi
> >
> > > > +
> > > > +Example
> > > > +-------
> > > > +
> > > > +Obtain basic information about SCMI server:
> > > > +
> > > > +::
> > > > +
> > > > +    => scmi base info
> > > > +    SCMI device: scmi
> > > > +      protocol version: 0x20000
> > > > +      # of agents: 3
> > > > +          0: platform
> > > > +        > 1: OSPM
> > > > +          2: PSCI
> > > > +      # of protocols: 4
> > > > +          Power domain management
> > > > +          Performance domain management
> > > > +          Clock management
> > > > +          Sensor management
> > > > +      vendor: Linaro
> > > > +      sub vendor: PMWG
> > > > +      impl version: 0x20b0000
> > > > +
> > > > +Ask for access permission to device#0:
> > > > +
> > > > +::
> > > > +
> > > > +    => scmi base perm_dev 1 0 1
> > > > +
> > > > +Reset configurations with all access permission settings retained:
> > > > +
> > > > +::
> > > > +
> > > > +    => scmi base reset 1 0
> > > > +
> > > > +Configuration
> > > > +-------------
> > > > +
> > > > +The scmi command is only available if CONFIG_CMD_SCMI=y.
> > > > +Default n because this command is mainly for debug purpose.
> > > > +
> > > > +Return value
> > > > +------------
> > > > +
> > > > +The return value ($?) is set to 0 if the operation succeeded,
> > > > +1 if the operation failed or -1 if the operation failed due to
> > > > +a syntax error.
> > > > --
> > > > 2.41.0
> > > >
> Regards,
> Simon
Simon Glass July 7, 2023, 5:35 p.m. UTC | #5
Hi,

On Tue, 4 Jul 2023 at 03:05, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, Jul 03, 2023 at 02:30:55PM +0100, Simon Glass wrote:
> > Hi ,
> >
> > On Mon, 3 Jul 2023 at 02:19, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > >
> > > On Thu, Jun 29, 2023 at 08:10:02PM +0100, Simon Glass wrote:
> > > > Hi AKASHI,
> > > >
> > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > > <takahiro.akashi@linaro.org> wrote:
> > > > >
> > > > > This is a help text for scmi command.
> > > > >
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > > ---
> > > > >  doc/usage/cmd/scmi.rst | 98 ++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 98 insertions(+)
> > > > >  create mode 100644 doc/usage/cmd/scmi.rst
> > > > >
> > > > > diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst
> > > > > new file mode 100644
> > > > > index 000000000000..20cdae4b877d
> > > > > --- /dev/null
> > > > > +++ b/doc/usage/cmd/scmi.rst
> > > > > @@ -0,0 +1,98 @@
> > > > > +.. SPDX-License-Identifier: GPL-2.0+:
> > > > > +
> > > > > +scmi command
> > > > > +============
> > > > > +
> > > > > +Synopsis
> > > > > +--------
> > > > > +
> > > > > +::
> > > > > +
> > > > > +    scmi base info
> > > > > +    scmi base perm_dev <agent id> <device id> <flags>
> > > > > +    scmi base perm_proto <agent id> <device id> <command id> <flags>
> > > > > +    scmi base reset <agent id> <flags>
> > > > > +
> > > > > +Description
> > > > > +-----------
> > > > > +
> > > > > +The scmi command is used to access and operate on SCMI server.
> > > > > +
> > > > > +scmi base info
> > > > > +~~~~~~~~~~~~~~
> > > > > +    Show base information about SCMI server and supported protocols
> > > > > +
> > > > > +scmi base perm_dev
> > > > > +~~~~~~~~~~~~~~~~~~
> > > > > +    Allow or deny access permission to the device
> > > > > +
> > > > > +scmi base perm_proto
> > > > > +~~~~~~~~~~~~~~~~~~~~
> > > > > +    Allow or deny access to the protocol on the device
> > > > > +
> > > > > +scmi base reset
> > > > > +~~~~~~~~~~~~~~~
> > > > > +    Reset the existing configurations
> > > > > +
> > > > > +Parameters are used as follows:
> > > > > +
> > > > > +<agent id>
> > > > > +    Agent ID
> > > >
> > > > what is this?
> > >
> > > I thought that the meaning was trivial in SCMI context.
> > > Will change it to "SCMI Agent ID".
> >
> > What is SCMI? Really you should explain and link to information about
> > things you mention in documentation. How else is anyone supposed to
>
> Generally yes, but please note that SCMI and related drivers are already
> there in U-Boot. For instance, see drivers/firmware/scmi/Kconfig.
> I don't think we need any more explanation about what is SCMI everywhere
> the word, "SCMI", appears.
>
> > figure out what you are talking about?
>
> Again, those who don't know about SCMI and if SCMI server is installed
> on their systems will never use this command.
>
> >
> > "SCMI Agent ID" doesn't tell us much. What form does it take? Is it a
> > string? How do you find it out?
>
> While I still believe that What SCMI agent ID means is trivial for
> those who read the SCMI specification, I will give a short description
> about possible values.
>
> > >
> > >
> > > > > +
> > > > > +<device id>
> > > > > +    Device ID
> > > >
> > > > what is this?
> > >
> > > Again, will change it to "SCMI Device ID".
> >
> > That doesn't help much. The purpose of documentation is to explain
> > things for people who don't already know it. We need to try to be as
> > helpful as possible.
>
> The same above.
> Please note that the definition of "device (ID)", except its data type,
> is out of scope of SCMI specification.
> It doesn't describe any means by which values be identified/retrieved.
>
> > >
> > > > > +
> > > > > +<command id>
> > > > > +    Protocol ID, should not be 0x10 (base protocol)
> > > >
> > > > what is this? Please add more detail
> > >
> > > Again, will change it to "SCMI Protocol ID".
> > > I think that users should be familiar with those terms
> > > if they want to use these interfaces.
> >
> > Maybe, but it still isn't clear what it is. A string?
>
> Will add a short description about its data type/format.
>
> > It is confusing
> > that the command ID is also a protocol ID.
>
> Yes, I know, but the confusion exists in SCMI specification.
> It sometimes seems to use both words almost interchangeably, even
> in a single context. For instance, the section 4.2.2.11 of [1]
> says,
>
> 4.2.2.11 BASE_SET_PROTOCOL_PERMISSIONS
>  ...
>  This command BASE_SET_PROTOCOL_PERMISSIONS is used to ...
>  (This "command" has a yet another meaning.)
>
> Parameters
> uint32_t command_id     Bits[31:8] Reserved, must be zero
>                         Bits[7:0] Protocol ID
>
> [1]  https://developer.arm.com/documentation/den0056/e/?lang=en
>
> So using "command_id" for a parameter name and "Protocol ID"
> as a (part of) value is very much aligned with the specification.
>
> That said, I will change "command_id" to "protocol_id" for now.
>
> > >
> > > > > +
> > > > > +<flags>
> > > > > +    Flags to control the action. See SCMI specification for
> > > > > +    defined values.
> > > >
> > > > ?
> > > >
> > > > Please add the flags here, or at the very least provide a URL and page
> > > > number, etc.
> > >
> > > I intentionally avoid providing details here because a set of flags
> > > acceptable to a specific SCMI server may depend on the server
> > > and its implementation version.
> > > The interface on U-Boot is just a wrapper to make a call to SCMI server
> > > via a transport layer and doesn't care what the parameters means.
> > > That said, I agree to referring to a URL to SCMI specification somewhere
> > > in this document.
> >
> > That will help. But also please add that this is a hex value (right?)
>
> Will do.

OK, well do what you can. Documentation which assumes a lot of new
knowledge is not very useful. Linking to docs is good but it still
helps to spell out acronyms at least once in each place, and add a
little summary about what SCMI is for. Linking to docs can be a pain
when the links disappear, also.

Regards,
Simon
diff mbox series

Patch

diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst
new file mode 100644
index 000000000000..20cdae4b877d
--- /dev/null
+++ b/doc/usage/cmd/scmi.rst
@@ -0,0 +1,98 @@ 
+.. SPDX-License-Identifier: GPL-2.0+:
+
+scmi command
+============
+
+Synopsis
+--------
+
+::
+
+    scmi base info
+    scmi base perm_dev <agent id> <device id> <flags>
+    scmi base perm_proto <agent id> <device id> <command id> <flags>
+    scmi base reset <agent id> <flags>
+
+Description
+-----------
+
+The scmi command is used to access and operate on SCMI server.
+
+scmi base info
+~~~~~~~~~~~~~~
+    Show base information about SCMI server and supported protocols
+
+scmi base perm_dev
+~~~~~~~~~~~~~~~~~~
+    Allow or deny access permission to the device
+
+scmi base perm_proto
+~~~~~~~~~~~~~~~~~~~~
+    Allow or deny access to the protocol on the device
+
+scmi base reset
+~~~~~~~~~~~~~~~
+    Reset the existing configurations
+
+Parameters are used as follows:
+
+<agent id>
+    Agent ID
+
+<device id>
+    Device ID
+
+<command id>
+    Protocol ID, should not be 0x10 (base protocol)
+
+<flags>
+    Flags to control the action. See SCMI specification for
+    defined values.
+
+Example
+-------
+
+Obtain basic information about SCMI server:
+
+::
+
+    => scmi base info
+    SCMI device: scmi
+      protocol version: 0x20000
+      # of agents: 3
+          0: platform
+        > 1: OSPM
+          2: PSCI
+      # of protocols: 4
+          Power domain management
+          Performance domain management
+          Clock management
+          Sensor management
+      vendor: Linaro
+      sub vendor: PMWG
+      impl version: 0x20b0000
+
+Ask for access permission to device#0:
+
+::
+
+    => scmi base perm_dev 1 0 1
+
+Reset configurations with all access permission settings retained:
+
+::
+
+    => scmi base reset 1 0
+
+Configuration
+-------------
+
+The scmi command is only available if CONFIG_CMD_SCMI=y.
+Default n because this command is mainly for debug purpose.
+
+Return value
+------------
+
+The return value ($?) is set to 0 if the operation succeeded,
+1 if the operation failed or -1 if the operation failed due to
+a syntax error.