Hi, On Tue, 4 Jul 2023 at 03:05, AKASHI Takahiro <takahiro.aka...@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.aka...@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.aka...@linaro.org> wrote: > > > > > > > > > > This is a help text for scmi command. > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@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