subject:"\[PATCH 08\/10\] cmd\: add scmi command for SCMI firmware"

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

2023-07-09 Thread AKASHI Takahiro

On Fri, Jul 07, 2023 at 11:35:52AM -0600, Simon Glass wrote:
> Hi,
> 
> On Tue, 4 Jul 2023 at 02:26, AKASHI Takahiro  
> wrote:
> >
> > On Mon, Jul 03, 2023 at 02:30:54PM +0100, Simon Glass wrote:
> > > Hi,
> > >
> > > On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro  
> > > wrote:
> > > >
> > > > On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> > > > > Hi AKASHI,
> > > > >
> > > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > > >  wrote:
> > > > > >
> > > > > > This command, "scmi", provides a command line interface to various 
> > > > > > SCMI
> > > > > > protocols. It supports at least initially SCMI base protocol with 
> > > > > > the sub
> > > > > > command, "base", and is intended mainly for debug purpose.
> > > > > >
> > > > > > Signed-off-by: AKASHI Takahiro 
> > > > > > ---
> > > > > >  cmd/Kconfig  |   8 ++
> > > > > >  cmd/Makefile |   1 +
> > > > > >  cmd/scmi.c   | 373 
> > > > > > +++
> > > > > >  3 files changed, 382 insertions(+)
> > > > > >  create mode 100644 cmd/scmi.c
> > > > > >
> > > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > > > index 02e54f1e50fe..065470a76295 100644
> > > > > > --- a/cmd/Kconfig
> > > > > > +++ b/cmd/Kconfig
> > > > > > @@ -2504,6 +2504,14 @@ 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
> > > > > > +   help
> > > > > > + This command provides user interfaces to several SCMI 
> > > > > > protocols,
> > > > > > + including Base protocol.
> > > > >
> > > > > Please mention what is SCMI
> > > >
> > > > I will give a full spelling.
> > > >
> > > > > >  endmenu
> > > > > >
> > > > > >  menu "Filesystem commands"
> > > > > > diff --git a/cmd/Makefile b/cmd/Makefile
> > > > > > index 6c37521b4e2b..826e0b74b587 100644
> > > > > > --- a/cmd/Makefile
> > > > > > +++ b/cmd/Makefile
> > > > > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> > > > > >  obj-$(CONFIG_CMD_NVME) += nvme.o
> > > > > >  obj-$(CONFIG_SANDBOX) += sb.o
> > > > > >  obj-$(CONFIG_CMD_SF) += sf.o
> > > > > > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> > > > > >  obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> > > > > >  obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> > > > > >  obj-$(CONFIG_CMD_SEAMA) += seama.o
> > > > > > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > > > > > new file mode 100644
> > > > > > index ..c97f77e97d95
> > > > > > --- /dev/null
> > > > > > +++ b/cmd/scmi.c
> > > > > > @@ -0,0 +1,373 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + *  SCMI utility command
> > > > > > + *
> > > > > > + *  Copyright (c) 2023 Linaro Limited
> > > > > > + * Author: AKASHI Takahiro
> > > > > > + */
> > > > > > +
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include  /* uclass_get_device */
> > > > >
> > > > > Goes before linux/bitfield.h
> > > >
> > > > Can you tell me why?
> > >
> > > It is just a convention:
> > > https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
> >
> > Okay.
> >
> > > >
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +
> > > > > > +static struct udevice *agent;
> > > > > > +static struct udevice *base_proto;
> > > > > > +static const struct scmi_base_ops *ops;
> > > > > > +
> > > > > > +struct {
> > > > > > +   enum scmi_std_protocol id;
> > > > > > +   const char *name;
> > > > > > +} protocol_name[] = {
> > > > > > +   {SCMI_PROTOCOL_ID_BASE, "Base"},
> > > > > > +   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > > > > > +   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > > > > > +   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > > > > > +   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > > > > > +   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > > > > > +   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > > > > > +   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain 
> > > > > > management"},
> > > > > > +};
> > > > > > +
> > > > > > +/**
> > > > > > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > > > > > + *
> > > > > > + * @id:Protocol ID
> > > > > > + *
> > > > > > + * Get the printable name of the protocol, @id
> > > > > > + *
> > > > > > + * Return: Name string on success, NULL on failure
> > > > > > + */
> > > > > > +static const char *scmi_convert_id_to_string(enum 
> > > > > > scmi_std_protocol id)
> > > > >
> > > > > scmi_lookup_id?
> > > >
> >

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

2023-07-07 Thread Simon Glass

Hi,

On Tue, 4 Jul 2023 at 02:26, AKASHI Takahiro  wrote:
>
> On Mon, Jul 03, 2023 at 02:30:54PM +0100, Simon Glass wrote:
> > Hi,
> >
> > On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro  
> > wrote:
> > >
> > > On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> > > > Hi AKASHI,
> > > >
> > > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > > >  wrote:
> > > > >
> > > > > This command, "scmi", provides a command line interface to various 
> > > > > SCMI
> > > > > protocols. It supports at least initially SCMI base protocol with the 
> > > > > sub
> > > > > command, "base", and is intended mainly for debug purpose.
> > > > >
> > > > > Signed-off-by: AKASHI Takahiro 
> > > > > ---
> > > > >  cmd/Kconfig  |   8 ++
> > > > >  cmd/Makefile |   1 +
> > > > >  cmd/scmi.c   | 373 
> > > > > +++
> > > > >  3 files changed, 382 insertions(+)
> > > > >  create mode 100644 cmd/scmi.c
> > > > >
> > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > > index 02e54f1e50fe..065470a76295 100644
> > > > > --- a/cmd/Kconfig
> > > > > +++ b/cmd/Kconfig
> > > > > @@ -2504,6 +2504,14 @@ 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
> > > > > +   help
> > > > > + This command provides user interfaces to several SCMI 
> > > > > protocols,
> > > > > + including Base protocol.
> > > >
> > > > Please mention what is SCMI
> > >
> > > I will give a full spelling.
> > >
> > > > >  endmenu
> > > > >
> > > > >  menu "Filesystem commands"
> > > > > diff --git a/cmd/Makefile b/cmd/Makefile
> > > > > index 6c37521b4e2b..826e0b74b587 100644
> > > > > --- a/cmd/Makefile
> > > > > +++ b/cmd/Makefile
> > > > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> > > > >  obj-$(CONFIG_CMD_NVME) += nvme.o
> > > > >  obj-$(CONFIG_SANDBOX) += sb.o
> > > > >  obj-$(CONFIG_CMD_SF) += sf.o
> > > > > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> > > > >  obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> > > > >  obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> > > > >  obj-$(CONFIG_CMD_SEAMA) += seama.o
> > > > > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > > > > new file mode 100644
> > > > > index ..c97f77e97d95
> > > > > --- /dev/null
> > > > > +++ b/cmd/scmi.c
> > > > > @@ -0,0 +1,373 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + *  SCMI utility command
> > > > > + *
> > > > > + *  Copyright (c) 2023 Linaro Limited
> > > > > + * Author: AKASHI Takahiro
> > > > > + */
> > > > > +
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include  /* uclass_get_device */
> > > >
> > > > Goes before linux/bitfield.h
> > >
> > > Can you tell me why?
> >
> > It is just a convention:
> > https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
>
> Okay.
>
> > >
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +
> > > > > +static struct udevice *agent;
> > > > > +static struct udevice *base_proto;
> > > > > +static const struct scmi_base_ops *ops;
> > > > > +
> > > > > +struct {
> > > > > +   enum scmi_std_protocol id;
> > > > > +   const char *name;
> > > > > +} protocol_name[] = {
> > > > > +   {SCMI_PROTOCOL_ID_BASE, "Base"},
> > > > > +   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > > > > +   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > > > > +   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > > > > +   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > > > > +   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > > > > +   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > > > > +   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain 
> > > > > management"},
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > > > > + *
> > > > > + * @id:Protocol ID
> > > > > + *
> > > > > + * Get the printable name of the protocol, @id
> > > > > + *
> > > > > + * Return: Name string on success, NULL on failure
> > > > > + */
> > > > > +static const char *scmi_convert_id_to_string(enum scmi_std_protocol 
> > > > > id)
> > > >
> > > > scmi_lookup_id?
> > >
> > > Not sure your intention.
> > > The name above gives us exactly what this function does for pretty 
> > > printing
> > > instead of a number.
> > > I think that 'lookup' implies we try to determine if the id exists or not.
> >
> > I am just trying to avoid the code turning into Java :-)
>
> Java?

Long names like stories

>
> > How about

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

2023-07-03 Thread AKASHI Takahiro

On Mon, Jul 03, 2023 at 02:30:54PM +0100, Simon Glass wrote:
> Hi,
> 
> On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro  
> wrote:
> >
> > On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> > > Hi AKASHI,
> > >
> > > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> > >  wrote:
> > > >
> > > > This command, "scmi", provides a command line interface to various SCMI
> > > > protocols. It supports at least initially SCMI base protocol with the 
> > > > sub
> > > > command, "base", and is intended mainly for debug purpose.
> > > >
> > > > Signed-off-by: AKASHI Takahiro 
> > > > ---
> > > >  cmd/Kconfig  |   8 ++
> > > >  cmd/Makefile |   1 +
> > > >  cmd/scmi.c   | 373 +++
> > > >  3 files changed, 382 insertions(+)
> > > >  create mode 100644 cmd/scmi.c
> > > >
> > > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > > index 02e54f1e50fe..065470a76295 100644
> > > > --- a/cmd/Kconfig
> > > > +++ b/cmd/Kconfig
> > > > @@ -2504,6 +2504,14 @@ 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
> > > > +   help
> > > > + This command provides user interfaces to several SCMI 
> > > > protocols,
> > > > + including Base protocol.
> > >
> > > Please mention what is SCMI
> >
> > I will give a full spelling.
> >
> > > >  endmenu
> > > >
> > > >  menu "Filesystem commands"
> > > > diff --git a/cmd/Makefile b/cmd/Makefile
> > > > index 6c37521b4e2b..826e0b74b587 100644
> > > > --- a/cmd/Makefile
> > > > +++ b/cmd/Makefile
> > > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> > > >  obj-$(CONFIG_CMD_NVME) += nvme.o
> > > >  obj-$(CONFIG_SANDBOX) += sb.o
> > > >  obj-$(CONFIG_CMD_SF) += sf.o
> > > > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> > > >  obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> > > >  obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> > > >  obj-$(CONFIG_CMD_SEAMA) += seama.o
> > > > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > > > new file mode 100644
> > > > index ..c97f77e97d95
> > > > --- /dev/null
> > > > +++ b/cmd/scmi.c
> > > > @@ -0,0 +1,373 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + *  SCMI utility command
> > > > + *
> > > > + *  Copyright (c) 2023 Linaro Limited
> > > > + * Author: AKASHI Takahiro
> > > > + */
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include  /* uclass_get_device */
> > >
> > > Goes before linux/bitfield.h
> >
> > Can you tell me why?
> 
> It is just a convention:
> https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files

Okay.

> >
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +static struct udevice *agent;
> > > > +static struct udevice *base_proto;
> > > > +static const struct scmi_base_ops *ops;
> > > > +
> > > > +struct {
> > > > +   enum scmi_std_protocol id;
> > > > +   const char *name;
> > > > +} protocol_name[] = {
> > > > +   {SCMI_PROTOCOL_ID_BASE, "Base"},
> > > > +   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > > > +   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > > > +   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > > > +   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > > > +   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > > > +   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > > > +   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
> > > > +};
> > > > +
> > > > +/**
> > > > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > > > + *
> > > > + * @id:Protocol ID
> > > > + *
> > > > + * Get the printable name of the protocol, @id
> > > > + *
> > > > + * Return: Name string on success, NULL on failure
> > > > + */
> > > > +static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
> > >
> > > scmi_lookup_id?
> >
> > Not sure your intention.
> > The name above gives us exactly what this function does for pretty printing
> > instead of a number.
> > I think that 'lookup' implies we try to determine if the id exists or not.
> 
> I am just trying to avoid the code turning into Java :-)

Java?

> How about scmi_get_name() ?

Well, more specifically scmi_get_protocol_name()?

-Takahiro Akashi

> Regards,
> Simon

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

2023-07-03 Thread Simon Glass

Hi,

On Mon, 3 Jul 2023 at 01:55, AKASHI Takahiro  wrote:
>
> On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
> >  wrote:
> > >
> > > This command, "scmi", provides a command line interface to various SCMI
> > > protocols. It supports at least initially SCMI base protocol with the sub
> > > command, "base", and is intended mainly for debug purpose.
> > >
> > > Signed-off-by: AKASHI Takahiro 
> > > ---
> > >  cmd/Kconfig  |   8 ++
> > >  cmd/Makefile |   1 +
> > >  cmd/scmi.c   | 373 +++
> > >  3 files changed, 382 insertions(+)
> > >  create mode 100644 cmd/scmi.c
> > >
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index 02e54f1e50fe..065470a76295 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -2504,6 +2504,14 @@ 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
> > > +   help
> > > + This command provides user interfaces to several SCMI protocols,
> > > + including Base protocol.
> >
> > Please mention what is SCMI
>
> I will give a full spelling.
>
> > >  endmenu
> > >
> > >  menu "Filesystem commands"
> > > diff --git a/cmd/Makefile b/cmd/Makefile
> > > index 6c37521b4e2b..826e0b74b587 100644
> > > --- a/cmd/Makefile
> > > +++ b/cmd/Makefile
> > > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> > >  obj-$(CONFIG_CMD_NVME) += nvme.o
> > >  obj-$(CONFIG_SANDBOX) += sb.o
> > >  obj-$(CONFIG_CMD_SF) += sf.o
> > > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> > >  obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> > >  obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> > >  obj-$(CONFIG_CMD_SEAMA) += seama.o
> > > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > > new file mode 100644
> > > index ..c97f77e97d95
> > > --- /dev/null
> > > +++ b/cmd/scmi.c
> > > @@ -0,0 +1,373 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + *  SCMI utility command
> > > + *
> > > + *  Copyright (c) 2023 Linaro Limited
> > > + * Author: AKASHI Takahiro
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include  /* uclass_get_device */
> >
> > Goes before linux/bitfield.h
>
> Can you tell me why?

It is just a convention:
https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files

>
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +static struct udevice *agent;
> > > +static struct udevice *base_proto;
> > > +static const struct scmi_base_ops *ops;
> > > +
> > > +struct {
> > > +   enum scmi_std_protocol id;
> > > +   const char *name;
> > > +} protocol_name[] = {
> > > +   {SCMI_PROTOCOL_ID_BASE, "Base"},
> > > +   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > > +   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > > +   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > > +   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > > +   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > > +   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > > +   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
> > > +};
> > > +
> > > +/**
> > > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > > + *
> > > + * @id:Protocol ID
> > > + *
> > > + * Get the printable name of the protocol, @id
> > > + *
> > > + * Return: Name string on success, NULL on failure
> > > + */
> > > +static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
> >
> > scmi_lookup_id?
>
> Not sure your intention.
> The name above gives us exactly what this function does for pretty printing
> instead of a number.
> I think that 'lookup' implies we try to determine if the id exists or not.

I am just trying to avoid the code turning into Java :-)

How about scmi_get_name() ?

Regards,
Simon

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

2023-07-02 Thread AKASHI Takahiro

On Thu, Jun 29, 2023 at 08:10:00PM +0100, Simon Glass wrote:
> Hi AKASHI,
> 
> On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
>  wrote:
> >
> > This command, "scmi", provides a command line interface to various SCMI
> > protocols. It supports at least initially SCMI base protocol with the sub
> > command, "base", and is intended mainly for debug purpose.
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  cmd/Kconfig  |   8 ++
> >  cmd/Makefile |   1 +
> >  cmd/scmi.c   | 373 +++
> >  3 files changed, 382 insertions(+)
> >  create mode 100644 cmd/scmi.c
> >
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 02e54f1e50fe..065470a76295 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2504,6 +2504,14 @@ 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
> > +   help
> > + This command provides user interfaces to several SCMI protocols,
> > + including Base protocol.
> 
> Please mention what is SCMI

I will give a full spelling.

> >  endmenu
> >
> >  menu "Filesystem commands"
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 6c37521b4e2b..826e0b74b587 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> >  obj-$(CONFIG_CMD_NVME) += nvme.o
> >  obj-$(CONFIG_SANDBOX) += sb.o
> >  obj-$(CONFIG_CMD_SF) += sf.o
> > +obj-$(CONFIG_CMD_SCMI) += scmi.o
> >  obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> >  obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> >  obj-$(CONFIG_CMD_SEAMA) += seama.o
> > diff --git a/cmd/scmi.c b/cmd/scmi.c
> > new file mode 100644
> > index ..c97f77e97d95
> > --- /dev/null
> > +++ b/cmd/scmi.c
> > @@ -0,0 +1,373 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  SCMI utility command
> > + *
> > + *  Copyright (c) 2023 Linaro Limited
> > + * Author: AKASHI Takahiro
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include  /* uclass_get_device */
> 
> Goes before linux/bitfield.h

Can you tell me why?

> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static struct udevice *agent;
> > +static struct udevice *base_proto;
> > +static const struct scmi_base_ops *ops;
> > +
> > +struct {
> > +   enum scmi_std_protocol id;
> > +   const char *name;
> > +} protocol_name[] = {
> > +   {SCMI_PROTOCOL_ID_BASE, "Base"},
> > +   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> > +   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> > +   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> > +   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> > +   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> > +   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> > +   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
> > +};
> > +
> > +/**
> > + * scmi_convert_id_to_string() - get the name of SCMI protocol
> > + *
> > + * @id:Protocol ID
> > + *
> > + * Get the printable name of the protocol, @id
> > + *
> > + * Return: Name string on success, NULL on failure
> > + */
> > +static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
> 
> scmi_lookup_id?

Not sure your intention.
The name above gives us exactly what this function does for pretty printing
instead of a number.
I think that 'lookup' implies we try to determine if the id exists or not.

> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < ARRAY_SIZE(protocol_name); i++)
> > +   if (id == protocol_name[i].id)
> > +   return protocol_name[i].name;
> > +
> > +   return NULL;
> > +}
> > +
> > +/**
> > + * do_scmi_base_info() - get the information of SCMI services
> > + *
> > + * @cmdtp: Command table
> > + * @flag:  Command flag
> > + * @argc:  Number of arguments
> > + * @argv:  Argument array
> > + *
> > + * Get the information of SCMI services using various interfaces
> > + * provided by the Base protocol.
> > + *
> > + * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
> > + */
> > +static int do_scmi_base_info(struct cmd_tbl *cmdtp, int flag, int argc,
> > +char * const argv[])
> > +{
> > +   u32 agent_id, num_protocols;
> > +   u8 agent_name[SCMI_BASE_NAME_LENGTH_MAX], *protocols;
> > +   int i, ret;
> > +
> > +   if (argc != 1)
> > +   return CMD_RET_USAGE;
> > +
> > +   printf("SCMI device: %s\n", agent->name);
> > +   printf("  protocol version: 0x%x\n", scmi_version(agent));
> > +   printf("  # of agents: %d\n", scmi_num_agents(agent));
> > +

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

2023-06-29 Thread Simon Glass

Hi AKASHI,

On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
 wrote:
>
> This command, "scmi", provides a command line interface to various SCMI
> protocols. It supports at least initially SCMI base protocol with the sub
> command, "base", and is intended mainly for debug purpose.
>
> Signed-off-by: AKASHI Takahiro 
> ---
>  cmd/Kconfig  |   8 ++
>  cmd/Makefile |   1 +
>  cmd/scmi.c   | 373 +++
>  3 files changed, 382 insertions(+)
>  create mode 100644 cmd/scmi.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 02e54f1e50fe..065470a76295 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2504,6 +2504,14 @@ 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
> +   help
> + This command provides user interfaces to several SCMI protocols,
> + including Base protocol.

Please mention what is SCMI

>  endmenu
>
>  menu "Filesystem commands"
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 6c37521b4e2b..826e0b74b587 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
>  obj-$(CONFIG_CMD_NVME) += nvme.o
>  obj-$(CONFIG_SANDBOX) += sb.o
>  obj-$(CONFIG_CMD_SF) += sf.o
> +obj-$(CONFIG_CMD_SCMI) += scmi.o
>  obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
>  obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
>  obj-$(CONFIG_CMD_SEAMA) += seama.o
> diff --git a/cmd/scmi.c b/cmd/scmi.c
> new file mode 100644
> index ..c97f77e97d95
> --- /dev/null
> +++ b/cmd/scmi.c
> @@ -0,0 +1,373 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  SCMI utility command
> + *
> + *  Copyright (c) 2023 Linaro Limited
> + * Author: AKASHI Takahiro
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include  /* uclass_get_device */

Goes before linux/bitfield.h

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static struct udevice *agent;
> +static struct udevice *base_proto;
> +static const struct scmi_base_ops *ops;
> +
> +struct {
> +   enum scmi_std_protocol id;
> +   const char *name;
> +} protocol_name[] = {
> +   {SCMI_PROTOCOL_ID_BASE, "Base"},
> +   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> +   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> +   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> +   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> +   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> +   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> +   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
> +};
> +
> +/**
> + * scmi_convert_id_to_string() - get the name of SCMI protocol
> + *
> + * @id:Protocol ID
> + *
> + * Get the printable name of the protocol, @id
> + *
> + * Return: Name string on success, NULL on failure
> + */
> +static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)

scmi_lookup_id?

> +{
> +   int i;
> +
> +   for (i = 0; i < ARRAY_SIZE(protocol_name); i++)
> +   if (id == protocol_name[i].id)
> +   return protocol_name[i].name;
> +
> +   return NULL;
> +}
> +
> +/**
> + * do_scmi_base_info() - get the information of SCMI services
> + *
> + * @cmdtp: Command table
> + * @flag:  Command flag
> + * @argc:  Number of arguments
> + * @argv:  Argument array
> + *
> + * Get the information of SCMI services using various interfaces
> + * provided by the Base protocol.
> + *
> + * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
> + */
> +static int do_scmi_base_info(struct cmd_tbl *cmdtp, int flag, int argc,
> +char * const argv[])
> +{
> +   u32 agent_id, num_protocols;
> +   u8 agent_name[SCMI_BASE_NAME_LENGTH_MAX], *protocols;
> +   int i, ret;
> +
> +   if (argc != 1)
> +   return CMD_RET_USAGE;
> +
> +   printf("SCMI device: %s\n", agent->name);
> +   printf("  protocol version: 0x%x\n", scmi_version(agent));
> +   printf("  # of agents: %d\n", scmi_num_agents(agent));
> +   for (i = 0; i < scmi_num_agents(agent); i++) {
> +   ret = (*ops->base_discover_agent)(base_proto, i, _id,
> + agent_name);
> +   if (ret) {
> +   if (ret != -EOPNOTSUPP)
> +   printf("base_discover_agent() failed for id: 
> %d (%d)\n",
> +  i, ret);
> +   break;
> +   }
> +   printf("%c%2d: %s\n", i == scmi_agent_id(agent) ? '>' : ' 
> ',
> +  i, agent_name);
> +

[PATCH 08/10] cmd: add scmi command for SCMI firmware

2023-06-27 Thread AKASHI Takahiro

This command, "scmi", provides a command line interface to various SCMI
protocols. It supports at least initially SCMI base protocol with the sub
command, "base", and is intended mainly for debug purpose.

Signed-off-by: AKASHI Takahiro 
---
 cmd/Kconfig  |   8 ++
 cmd/Makefile |   1 +
 cmd/scmi.c   | 373 +++
 3 files changed, 382 insertions(+)
 create mode 100644 cmd/scmi.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 02e54f1e50fe..065470a76295 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2504,6 +2504,14 @@ 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
+   help
+ This command provides user interfaces to several SCMI protocols,
+ including Base protocol.
 endmenu
 
 menu "Filesystem commands"
diff --git a/cmd/Makefile b/cmd/Makefile
index 6c37521b4e2b..826e0b74b587 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
 obj-$(CONFIG_CMD_NVME) += nvme.o
 obj-$(CONFIG_SANDBOX) += sb.o
 obj-$(CONFIG_CMD_SF) += sf.o
+obj-$(CONFIG_CMD_SCMI) += scmi.o
 obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
 obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
 obj-$(CONFIG_CMD_SEAMA) += seama.o
diff --git a/cmd/scmi.c b/cmd/scmi.c
new file mode 100644
index ..c97f77e97d95
--- /dev/null
+++ b/cmd/scmi.c
@@ -0,0 +1,373 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  SCMI utility command
+ *
+ *  Copyright (c) 2023 Linaro Limited
+ * Author: AKASHI Takahiro
+ */
+
+#include 
+#include 
+#include 
+#include  /* uclass_get_device */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static struct udevice *agent;
+static struct udevice *base_proto;
+static const struct scmi_base_ops *ops;
+
+struct {
+   enum scmi_std_protocol id;
+   const char *name;
+} protocol_name[] = {
+   {SCMI_PROTOCOL_ID_BASE, "Base"},
+   {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
+   {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
+   {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
+   {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
+   {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
+   {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
+   {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
+};
+
+/**
+ * scmi_convert_id_to_string() - get the name of SCMI protocol
+ *
+ * @id:Protocol ID
+ *
+ * Get the printable name of the protocol, @id
+ *
+ * Return: Name string on success, NULL on failure
+ */
+static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(protocol_name); i++)
+   if (id == protocol_name[i].id)
+   return protocol_name[i].name;
+
+   return NULL;
+}
+
+/**
+ * do_scmi_base_info() - get the information of SCMI services
+ *
+ * @cmdtp: Command table
+ * @flag:  Command flag
+ * @argc:  Number of arguments
+ * @argv:  Argument array
+ *
+ * Get the information of SCMI services using various interfaces
+ * provided by the Base protocol.
+ *
+ * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ */
+static int do_scmi_base_info(struct cmd_tbl *cmdtp, int flag, int argc,
+char * const argv[])
+{
+   u32 agent_id, num_protocols;
+   u8 agent_name[SCMI_BASE_NAME_LENGTH_MAX], *protocols;
+   int i, ret;
+
+   if (argc != 1)
+   return CMD_RET_USAGE;
+
+   printf("SCMI device: %s\n", agent->name);
+   printf("  protocol version: 0x%x\n", scmi_version(agent));
+   printf("  # of agents: %d\n", scmi_num_agents(agent));
+   for (i = 0; i < scmi_num_agents(agent); i++) {
+   ret = (*ops->base_discover_agent)(base_proto, i, _id,
+ agent_name);
+   if (ret) {
+   if (ret != -EOPNOTSUPP)
+   printf("base_discover_agent() failed for id: %d 
(%d)\n",
+  i, ret);
+   break;
+   }
+   printf("%c%2d: %s\n", i == scmi_agent_id(agent) ? '>' : ' ',
+  i, agent_name);
+   }
+   printf("  # of protocols: %d\n", scmi_num_protocols(agent));
+   num_protocols = scmi_num_protocols(agent);
+   protocols = scmi_protocols(agent);
+   if (protocols)
+   for (i = 0; i < num_protocols; i++)
+   printf("  %s\n",
+  scmi_convert_id_to_string(protocols[i]));
+   printf("  vendor: %s\n", scmi_vendor(agent));
+   printf("  sub

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware

[PATCH 08/10] cmd: add scmi command for SCMI firmware

7 matches

Site Navigation

Mail list logo

Footer information