diff mbox series

[V5,1/2] firmware: arm_scmi: Add QCOM Generic Vendor Protocol documentation

Message ID 20241115011515.1313447-2-quic_sibis@quicinc.com
State New
Headers show
Series [V5,1/2] firmware: arm_scmi: Add QCOM Generic Vendor Protocol documentation | expand

Commit Message

Sibi Sankar Nov. 15, 2024, 1:15 a.m. UTC
Add QCOM System Control Management Interface (SCMI) Generic Vendor
Extensions Protocol documentation.

Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---

v4:
* Update the protol attributes doc with more information. [Cristian]

 .../arm_scmi/vendors/qcom/qcom_generic.rst    | 211 ++++++++++++++++++
 1 file changed, 211 insertions(+)
 create mode 100644 drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst

Comments

Cristian Marussi Dec. 4, 2024, 11:48 a.m. UTC | #1
On Fri, Nov 15, 2024 at 06:45:14AM +0530, Sibi Sankar wrote:
> Add QCOM System Control Management Interface (SCMI) Generic Vendor
> Extensions Protocol documentation.
> 
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
> 
> v4:
> * Update the protol attributes doc with more information. [Cristian]
> 
>  .../arm_scmi/vendors/qcom/qcom_generic.rst    | 211 ++++++++++++++++++
>  1 file changed, 211 insertions(+)
>  create mode 100644 drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> 
> diff --git a/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> new file mode 100644
> index 000000000000..141bc932e30f
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> @@ -0,0 +1,211 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. include:: <isonum.txt>
> +
> +===============================================================================
> +QCOM System Control and Management Interface(SCMI) Vendor Protocols Extension
> +===============================================================================
> +
> +:Copyright: |copy| 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> +
> +:Author: Sibi Sankar <quic_sibis@quicinc.com>
> +
> +SCMI_GENERIC: System Control and Management Interface QCOM Generic Vendor Protocol
> +==================================================================================
> +
> +This protocol is intended as a generic way of exposing a number of Qualcomm
> +SoC specific features through a mixture of pre-determined algorithm string and
> +param_id pairs hosted on the SCMI controller. It implements an interface compliant
> +with the Arm SCMI Specification with additional vendor specific commands as
> +detailed below.
> +
> +Commands:
> +_________
> +
> +PROTOCOL_VERSION
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x0
> +protocol_id: 0x80
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values                                                                 |
> ++---------------+--------------------------------------------------------------+
> +|Name           |Description                                                   |
> ++---------------+--------------------------------------------------------------+
> +|int32 status   |See ARM SCMI Specification for status code definitions.       |
> ++---------------+--------------------------------------------------------------+
> +|uint32 version |For this revision of the specification, this value must be    |
> +|               |0x10000.                                                      |
> ++---------------+--------------------------------------------------------------+
> +
> +PROTOCOL_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x1
> +protocol_id: 0x80
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values                                                                 |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|int32 status      |See ARM SCMI Specification for status code definitions.    |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |Bits[31:16] Reserved, must be to 0.                        |
> +|                  |Bits[15:8] Number of agents in the system                  |
> +|                  |Bits[7:0] Number of vendor protocols in the system         |
> ++------------------+-----------------------------------------------------------+

Thanks of clarifing this....may I ask why number of agents is reported
here too given that it is already exposed by Base protocol ?

Not really arguing about this so much, but you will end up having to maintain this
on 2 different protocols fw side...or are they not 'agents' in the SCMI meaning ?

Anyway, I'm fine with it, even though you dont seem to use this
anywhere.

> +
> +PROTOCOL_MESSAGE_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x2
> +protocol_id: 0x80
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values                                                                 |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|int32 status      |See ARM SCMI Specification for status code definitions.    |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |For all message id's the parameter has a value of 0.       |
> ++------------------+-----------------------------------------------------------+
> +
> +QCOM_SCMI_SET_PARAM
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x10
> +protocol_id: 0x80
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters                                                                    |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 ext_id     |Reserved, must be zero.                                    |
> ++------------------+-----------------------------------------------------------+
> +|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 param_id   |Serves as the token message id for the algorithm string    |
> +|                  |and is used to set various parameters supported by it.     |
> ++------------------+-----------------------------------------------------------+
> +|uint32 buf[]      |Serves as the payload for the specified param_id and       |
> +|                  |algorithm string pair.                                     |
> ++------------------+-----------------------------------------------------------+
> +|Return values                                                                 |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
> +|                  |by the chosen algorithm string.                            |
> +|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
> +|                  |matches.                                                   |
> +|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
> +|                  |is rejected by the algorithm string.                       |
> ++------------------+-----------------------------------------------------------+
> +
> +QCOM_SCMI_GET_PARAM
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x11
> +protocol_id: 0x80
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters                                                                    |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 ext_id     |Reserved, must be zero.                                    |
> ++------------------+-----------------------------------------------------------+
> +|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 param_id   |Serves as the token message id for the algorithm string.   |
> ++------------------+-----------------------------------------------------------+
> +|uint32 buf[]      |Serves as the payload and store of value for the specified |
> +|                  |param_id and algorithm string pair.                        |
> ++------------------+-----------------------------------------------------------+
> +|Return values                                                                 |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
> +|                  |by the chosen algorithm string and the result is copied    |
> +|                  |into buf[].                                                |
> +|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
> +|                  |matches.                                                   |
> +|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
> +|                  |is rejected by the algorithm string.                       |
> ++------------------+-----------------------------------------------------------+

..missed this last time...so you should add here also a field describing
the reply buf right ? (as it happenns really in your code) something
like:

 ++------------------+-----------------------------------------------------------+
 +|uint32 buf[]      |Holds the payload of the result of the query.	 	 |
 +|                  |					                         |
 ++------------------+-----------------------------------------------------------+

Thanks,
Cristian
Sibi Sankar Dec. 5, 2024, 11:07 a.m. UTC | #2
On 12/4/24 17:18, Cristian Marussi wrote:
> On Fri, Nov 15, 2024 at 06:45:14AM +0530, Sibi Sankar wrote:
>> Add QCOM System Control Management Interface (SCMI) Generic Vendor
>> Extensions Protocol documentation.
>>
>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
>> ---
>>
>> v4:
>> * Update the protol attributes doc with more information. [Cristian]
>>
>>   .../arm_scmi/vendors/qcom/qcom_generic.rst    | 211 ++++++++++++++++++
>>   1 file changed, 211 insertions(+)
>>   create mode 100644 drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
>>
>> diff --git a/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
>> new file mode 100644
>> index 000000000000..141bc932e30f
>> --- /dev/null
>> +++ b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
>> @@ -0,0 +1,211 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +.. include:: <isonum.txt>
>> +
>> +===============================================================================
>> +QCOM System Control and Management Interface(SCMI) Vendor Protocols Extension
>> +===============================================================================
>> +
>> +:Copyright: |copy| 2024, Qualcomm Innovation Center, Inc. All rights reserved.
>> +
>> +:Author: Sibi Sankar <quic_sibis@quicinc.com>
>> +
>> +SCMI_GENERIC: System Control and Management Interface QCOM Generic Vendor Protocol
>> +==================================================================================
>> +
>> +This protocol is intended as a generic way of exposing a number of Qualcomm
>> +SoC specific features through a mixture of pre-determined algorithm string and
>> +param_id pairs hosted on the SCMI controller. It implements an interface compliant
>> +with the Arm SCMI Specification with additional vendor specific commands as
>> +detailed below.
>> +
>> +Commands:
>> +_________
>> +
>> +PROTOCOL_VERSION
>> +~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x0
>> +protocol_id: 0x80
>> +
>> ++---------------+--------------------------------------------------------------+
>> +|Return values                                                                 |
>> ++---------------+--------------------------------------------------------------+
>> +|Name           |Description                                                   |
>> ++---------------+--------------------------------------------------------------+
>> +|int32 status   |See ARM SCMI Specification for status code definitions.       |
>> ++---------------+--------------------------------------------------------------+
>> +|uint32 version |For this revision of the specification, this value must be    |
>> +|               |0x10000.                                                      |
>> ++---------------+--------------------------------------------------------------+
>> +
>> +PROTOCOL_ATTRIBUTES
>> +~~~~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x1
>> +protocol_id: 0x80
>> +
>> ++---------------+--------------------------------------------------------------+
>> +|Return values                                                                 |
>> ++------------------+-----------------------------------------------------------+
>> +|Name              |Description                                                |
>> ++------------------+-----------------------------------------------------------+
>> +|int32 status      |See ARM SCMI Specification for status code definitions.    |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 attributes |Bits[31:16] Reserved, must be to 0.                        |
>> +|                  |Bits[15:8] Number of agents in the system                  |
>> +|                  |Bits[7:0] Number of vendor protocols in the system         |
>> ++------------------+-----------------------------------------------------------+
> 
> Thanks of clarifing this....may I ask why number of agents is reported
> here too given that it is already exposed by Base protocol ?
> 
> Not really arguing about this so much, but you will end up having to maintain this
> on 2 different protocols fw side...or are they not 'agents' in the SCMI meaning ?
> 
> Anyway, I'm fine with it, even though you dont seem to use this
> anywhere.

We don't use it anywhere and it looks like it was just put together
so that this protocol is compliant to the spec :|

> 
>> +
>> +PROTOCOL_MESSAGE_ATTRIBUTES
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x2
>> +protocol_id: 0x80
>> +
>> ++---------------+--------------------------------------------------------------+
>> +|Return values                                                                 |
>> ++------------------+-----------------------------------------------------------+
>> +|Name              |Description                                                |
>> ++------------------+-----------------------------------------------------------+
>> +|int32 status      |See ARM SCMI Specification for status code definitions.    |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 attributes |For all message id's the parameter has a value of 0.       |
>> ++------------------+-----------------------------------------------------------+
>> +
>> +QCOM_SCMI_SET_PARAM
>> +~~~~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x10
>> +protocol_id: 0x80
>> +
>> ++------------------+-----------------------------------------------------------+
>> +|Parameters                                                                    |
>> ++------------------+-----------------------------------------------------------+
>> +|Name              |Description                                                |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 ext_id     |Reserved, must be zero.                                    |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 param_id   |Serves as the token message id for the algorithm string    |
>> +|                  |and is used to set various parameters supported by it.     |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 buf[]      |Serves as the payload for the specified param_id and       |
>> +|                  |algorithm string pair.                                     |
>> ++------------------+-----------------------------------------------------------+
>> +|Return values                                                                 |
>> ++------------------+-----------------------------------------------------------+
>> +|Name              |Description                                                |
>> ++------------------+-----------------------------------------------------------+
>> +|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
>> +|                  |by the chosen algorithm string.                            |
>> +|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
>> +|                  |matches.                                                   |
>> +|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
>> +|                  |is rejected by the algorithm string.                       |
>> ++------------------+-----------------------------------------------------------+
>> +
>> +QCOM_SCMI_GET_PARAM
>> +~~~~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x11
>> +protocol_id: 0x80
>> +
>> ++------------------+-----------------------------------------------------------+
>> +|Parameters                                                                    |
>> ++------------------+-----------------------------------------------------------+
>> +|Name              |Description                                                |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 ext_id     |Reserved, must be zero.                                    |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 param_id   |Serves as the token message id for the algorithm string.   |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 buf[]      |Serves as the payload and store of value for the specified |
>> +|                  |param_id and algorithm string pair.                        |
>> ++------------------+-----------------------------------------------------------+
>> +|Return values                                                                 |
>> ++------------------+-----------------------------------------------------------+
>> +|Name              |Description                                                |
>> ++------------------+-----------------------------------------------------------+
>> +|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
>> +|                  |by the chosen algorithm string and the result is copied    |
>> +|                  |into buf[].                                                |
>> +|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
>> +|                  |matches.                                                   |
>> +|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
>> +|                  |is rejected by the algorithm string.                       |
>> ++------------------+-----------------------------------------------------------+
> 
> ..missed this last time...so you should add here also a field describing
> the reply buf right ? (as it happenns really in your code) something
> like:
> 
>   ++------------------+-----------------------------------------------------------+
>   +|uint32 buf[]      |Holds the payload of the result of the query.	 	 |
>   +|                  |					                         |
>   ++------------------+-----------------------------------------------------------+

Thanks will get this added in the next re-spin.

-Sibi

> 
> Thanks,
> Cristian
Cristian Marussi Dec. 5, 2024, 11:37 a.m. UTC | #3
On Thu, Dec 05, 2024 at 04:37:28PM +0530, Sibi Sankar wrote:
> 
> 
> On 12/4/24 17:18, Cristian Marussi wrote:
> > On Fri, Nov 15, 2024 at 06:45:14AM +0530, Sibi Sankar wrote:
> > > Add QCOM System Control Management Interface (SCMI) Generic Vendor
> > > Extensions Protocol documentation.
> > > 
> > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> > > ---
> > > 
> > > v4:
> > > * Update the protol attributes doc with more information. [Cristian]
> > > 
> > >   .../arm_scmi/vendors/qcom/qcom_generic.rst    | 211 ++++++++++++++++++
> > >   1 file changed, 211 insertions(+)
> > >   create mode 100644 drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> > > 
> > > diff --git a/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> > > new file mode 100644
> > > index 000000000000..141bc932e30f
> > > --- /dev/null
> > > +++ b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> > > @@ -0,0 +1,211 @@
> > > +.. SPDX-License-Identifier: GPL-2.0
> > > +.. include:: <isonum.txt>
> > > +
> > > +===============================================================================
> > > +QCOM System Control and Management Interface(SCMI) Vendor Protocols Extension
> > > +===============================================================================
> > > +
> > > +:Copyright: |copy| 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> > > +
> > > +:Author: Sibi Sankar <quic_sibis@quicinc.com>
> > > +
> > > +SCMI_GENERIC: System Control and Management Interface QCOM Generic Vendor Protocol
> > > +==================================================================================
> > > +
> > > +This protocol is intended as a generic way of exposing a number of Qualcomm
> > > +SoC specific features through a mixture of pre-determined algorithm string and
> > > +param_id pairs hosted on the SCMI controller. It implements an interface compliant
> > > +with the Arm SCMI Specification with additional vendor specific commands as
> > > +detailed below.
> > > +
> > > +Commands:
> > > +_________
> > > +
> > > +PROTOCOL_VERSION
> > > +~~~~~~~~~~~~~~~~
> > > +
> > > +message_id: 0x0
> > > +protocol_id: 0x80
> > > +
> > > ++---------------+--------------------------------------------------------------+
> > > +|Return values                                                                 |
> > > ++---------------+--------------------------------------------------------------+
> > > +|Name           |Description                                                   |
> > > ++---------------+--------------------------------------------------------------+
> > > +|int32 status   |See ARM SCMI Specification for status code definitions.       |
> > > ++---------------+--------------------------------------------------------------+
> > > +|uint32 version |For this revision of the specification, this value must be    |
> > > +|               |0x10000.                                                      |
> > > ++---------------+--------------------------------------------------------------+
> > > +
> > > +PROTOCOL_ATTRIBUTES
> > > +~~~~~~~~~~~~~~~~~~~
> > > +
> > > +message_id: 0x1
> > > +protocol_id: 0x80
> > > +
> > > ++---------------+--------------------------------------------------------------+
> > > +|Return values                                                                 |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Name              |Description                                                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|int32 status      |See ARM SCMI Specification for status code definitions.    |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 attributes |Bits[31:16] Reserved, must be to 0.                        |
> > > +|                  |Bits[15:8] Number of agents in the system                  |
> > > +|                  |Bits[7:0] Number of vendor protocols in the system         |
> > > ++------------------+-----------------------------------------------------------+
> > 
> > Thanks of clarifing this....may I ask why number of agents is reported
> > here too given that it is already exposed by Base protocol ?
> > 
> > Not really arguing about this so much, but you will end up having to maintain this
> > on 2 different protocols fw side...or are they not 'agents' in the SCMI meaning ?
> > 
> > Anyway, I'm fine with it, even though you dont seem to use this
> > anywhere.
> 
> We don't use it anywhere and it looks like it was just put together
> so that this protocol is compliant to the spec :|
> 

mmm...you mean the vendor protocol is compliant with this vendor doc ?

...because while PROTOCOL_ATTRIBUTES(0x1) is a mandatory command for
each protocols (including vendors) there isn't any expectation from the spec to
expose such info like agents and vendors protos....it is expected to
expose attributes specific to the protocol itself...or nothing if you
dont need anything.
 
> > 
> > > +
> > > +PROTOCOL_MESSAGE_ATTRIBUTES
> > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > +
> > > +message_id: 0x2
> > > +protocol_id: 0x80
> > > +
> > > ++---------------+--------------------------------------------------------------+
> > > +|Return values                                                                 |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Name              |Description                                                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|int32 status      |See ARM SCMI Specification for status code definitions.    |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 attributes |For all message id's the parameter has a value of 0.       |
> > > ++------------------+-----------------------------------------------------------+
> > > +
> > > +QCOM_SCMI_SET_PARAM
> > > +~~~~~~~~~~~~~~~~~~~
> > > +
> > > +message_id: 0x10
> > > +protocol_id: 0x80
> > > +
> > > ++------------------+-----------------------------------------------------------+
> > > +|Parameters                                                                    |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Name              |Description                                                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 ext_id     |Reserved, must be zero.                                    |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 param_id   |Serves as the token message id for the algorithm string    |
> > > +|                  |and is used to set various parameters supported by it.     |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 buf[]      |Serves as the payload for the specified param_id and       |
> > > +|                  |algorithm string pair.                                     |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Return values                                                                 |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Name              |Description                                                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
> > > +|                  |by the chosen algorithm string.                            |
> > > +|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
> > > +|                  |matches.                                                   |
> > > +|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
> > > +|                  |is rejected by the algorithm string.                       |
> > > ++------------------+-----------------------------------------------------------+
> > > +
> > > +QCOM_SCMI_GET_PARAM
> > > +~~~~~~~~~~~~~~~~~~~
> > > +
> > > +message_id: 0x11
> > > +protocol_id: 0x80
> > > +
> > > ++------------------+-----------------------------------------------------------+
> > > +|Parameters                                                                    |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Name              |Description                                                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 ext_id     |Reserved, must be zero.                                    |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 param_id   |Serves as the token message id for the algorithm string.   |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 buf[]      |Serves as the payload and store of value for the specified |
> > > +|                  |param_id and algorithm string pair.                        |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Return values                                                                 |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Name              |Description                                                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
> > > +|                  |by the chosen algorithm string and the result is copied    |
> > > +|                  |into buf[].                                                |
> > > +|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
> > > +|                  |matches.                                                   |
> > > +|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
> > > +|                  |is rejected by the algorithm string.                       |
> > > ++------------------+-----------------------------------------------------------+
> > 
> > ..missed this last time...so you should add here also a field describing
> > the reply buf right ? (as it happenns really in your code) something
> > like:
> > 
> >   ++------------------+-----------------------------------------------------------+
> >   +|uint32 buf[]      |Holds the payload of the result of the query.	 	 |
> >   +|                  |					                         |
> >   ++------------------+-----------------------------------------------------------+
> 
> Thanks will get this added in the next re-spin.
>

Ok.

Thanks,
Cristian
Sudeep Holla Dec. 5, 2024, 3:32 p.m. UTC | #4
On Thu, Dec 05, 2024 at 04:37:28PM +0530, Sibi Sankar wrote:
> 
> 
> On 12/4/24 17:18, Cristian Marussi wrote:
> > On Fri, Nov 15, 2024 at 06:45:14AM +0530, Sibi Sankar wrote:
> > > Add QCOM System Control Management Interface (SCMI) Generic Vendor
> > > Extensions Protocol documentation.
> > > 
> > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> > > ---
> > > 
> > > v4:
> > > * Update the protol attributes doc with more information. [Cristian]
> > > 
> > >   .../arm_scmi/vendors/qcom/qcom_generic.rst    | 211 ++++++++++++++++++
> > >   1 file changed, 211 insertions(+)
> > >   create mode 100644 drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> > > 
> > > diff --git a/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> > > new file mode 100644
> > > index 000000000000..141bc932e30f
> > > --- /dev/null
> > > +++ b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> > > @@ -0,0 +1,211 @@
> > > +.. SPDX-License-Identifier: GPL-2.0
> > > +.. include:: <isonum.txt>
> > > +
> > > +===============================================================================
> > > +QCOM System Control and Management Interface(SCMI) Vendor Protocols Extension
> > > +===============================================================================
> > > +
> > > +:Copyright: |copy| 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> > > +
> > > +:Author: Sibi Sankar <quic_sibis@quicinc.com>
> > > +
> > > +SCMI_GENERIC: System Control and Management Interface QCOM Generic Vendor Protocol
> > > +==================================================================================
> > > +
> > > +This protocol is intended as a generic way of exposing a number of Qualcomm
> > > +SoC specific features through a mixture of pre-determined algorithm string and
> > > +param_id pairs hosted on the SCMI controller. It implements an interface compliant
> > > +with the Arm SCMI Specification with additional vendor specific commands as
> > > +detailed below.
> > > +
> > > +Commands:
> > > +_________
> > > +
> > > +PROTOCOL_VERSION
> > > +~~~~~~~~~~~~~~~~
> > > +
> > > +message_id: 0x0
> > > +protocol_id: 0x80
> > > +
> > > ++---------------+--------------------------------------------------------------+
> > > +|Return values                                                                 |
> > > ++---------------+--------------------------------------------------------------+
> > > +|Name           |Description                                                   |
> > > ++---------------+--------------------------------------------------------------+
> > > +|int32 status   |See ARM SCMI Specification for status code definitions.       |
> > > ++---------------+--------------------------------------------------------------+
> > > +|uint32 version |For this revision of the specification, this value must be    |
> > > +|               |0x10000.                                                      |
> > > ++---------------+--------------------------------------------------------------+
> > > +
> > > +PROTOCOL_ATTRIBUTES
> > > +~~~~~~~~~~~~~~~~~~~
> > > +
> > > +message_id: 0x1
> > > +protocol_id: 0x80
> > > +
> > > ++---------------+--------------------------------------------------------------+
> > > +|Return values                                                                 |
> > > ++------------------+-----------------------------------------------------------+
> > > +|Name              |Description                                                |
> > > ++------------------+-----------------------------------------------------------+
> > > +|int32 status      |See ARM SCMI Specification for status code definitions.    |
> > > ++------------------+-----------------------------------------------------------+
> > > +|uint32 attributes |Bits[31:16] Reserved, must be to 0.                        |
> > > +|                  |Bits[15:8] Number of agents in the system                  |
> > > +|                  |Bits[7:0] Number of vendor protocols in the system         |
> > > ++------------------+-----------------------------------------------------------+
> > 
> > Thanks of clarifing this....may I ask why number of agents is reported
> > here too given that it is already exposed by Base protocol ?
> > 
> > Not really arguing about this so much, but you will end up having to maintain this
> > on 2 different protocols fw side...or are they not 'agents' in the SCMI meaning ?
> > 
> > Anyway, I'm fine with it, even though you dont seem to use this
> > anywhere.
> 
> We don't use it anywhere and it looks like it was just put together
> so that this protocol is compliant to the spec :|

Interesting, I need to dig and check if that is needed to be compliant.
Even if it is required, please add a text to say it must match what std.
BASE protocol response(IOW it just can't be random here as you don't use
it anymore). We may decide to match and warn if required at all.
Sudeep Holla Dec. 5, 2024, 3:53 p.m. UTC | #5
On Fri, Nov 15, 2024 at 06:45:14AM +0530, Sibi Sankar wrote:
> Add QCOM System Control Management Interface (SCMI) Generic Vendor
> Extensions Protocol documentation.
> 
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
> 
> v4:
> * Update the protol attributes doc with more information. [Cristian]
> 
>  .../arm_scmi/vendors/qcom/qcom_generic.rst    | 211 ++++++++++++++++++
>  1 file changed, 211 insertions(+)
>  create mode 100644 drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> 
> diff --git a/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> new file mode 100644
> index 000000000000..141bc932e30f
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
> @@ -0,0 +1,211 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. include:: <isonum.txt>
> +
> +===============================================================================
> +QCOM System Control and Management Interface(SCMI) Vendor Protocols Extension
> +===============================================================================
> +
> +:Copyright: |copy| 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> +
> +:Author: Sibi Sankar <quic_sibis@quicinc.com>
> +
> +SCMI_GENERIC: System Control and Management Interface QCOM Generic Vendor Protocol
> +==================================================================================
> +
> +This protocol is intended as a generic way of exposing a number of Qualcomm
> +SoC specific features through a mixture of pre-determined algorithm string and
> +param_id pairs hosted on the SCMI controller. It implements an interface compliant
> +with the Arm SCMI Specification with additional vendor specific commands as
> +detailed below.
> +
> +Commands:
> +_________
> +
> +PROTOCOL_VERSION
> +~~~~~~~~~~~~~~~~
> +
> +message_id: 0x0
> +protocol_id: 0x80
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values                                                                 |
> ++---------------+--------------------------------------------------------------+
> +|Name           |Description                                                   |
> ++---------------+--------------------------------------------------------------+
> +|int32 status   |See ARM SCMI Specification for status code definitions.       |
> ++---------------+--------------------------------------------------------------+
> +|uint32 version |For this revision of the specification, this value must be    |
> +|               |0x10000.                                                      |
> ++---------------+--------------------------------------------------------------+
> +
> +PROTOCOL_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x1
> +protocol_id: 0x80
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values                                                                 |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|int32 status      |See ARM SCMI Specification for status code definitions.    |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |Bits[31:16] Reserved, must be to 0.                        |
> +|                  |Bits[15:8] Number of agents in the system                  |
> +|                  |Bits[7:0] Number of vendor protocols in the system         |
> ++------------------+-----------------------------------------------------------+
> +
> +PROTOCOL_MESSAGE_ATTRIBUTES
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +message_id: 0x2
> +protocol_id: 0x80
> +
> ++---------------+--------------------------------------------------------------+
> +|Return values                                                                 |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|int32 status      |See ARM SCMI Specification for status code definitions.    |
> ++------------------+-----------------------------------------------------------+
> +|uint32 attributes |For all message id's the parameter has a value of 0.       |
> ++------------------+-----------------------------------------------------------+
> +
> +QCOM_SCMI_SET_PARAM
> +~~~~~~~~~~~~~~~~~~~
>

I can understand the missing description for the above commands, but for the
list below, no. What does QCOM_SCMI_SET_PARAM do exactly ? All I can
understand is the syntax of the interface with below details. What is the
algorithm string ? Is it fixed or choice of the caller ? If fixed, can we
have that list here ?

> +message_id: 0x10
> +protocol_id: 0x80
> +
> ++------------------+-----------------------------------------------------------+
> +|Parameters                                                                    |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 ext_id     |Reserved, must be zero.                                    |
> ++------------------+-----------------------------------------------------------+
> +|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
> ++------------------+-----------------------------------------------------------+
> +|uint32 param_id   |Serves as the token message id for the algorithm string    |
> +|                  |and is used to set various parameters supported by it.     |

And how is it related to the algorithm string ? Sorry details please. Based on
the quality firmware we have seen so far, I will be more pedantic, you need to
be patient to make any progress and I don't want to deal with the Qcom
firmware mess with all these. I will make you specify every single detail and
the code will just be compliant with that. Anything else will be firmware bug
that needs to be fixed in the firmware. Sorry, I don't see any other approach
will make anyone's life easier here.

> ++------------------+-----------------------------------------------------------+
> +|uint32 buf[]      |Serves as the payload for the specified param_id and       |
> +|                  |algorithm string pair.                                     |
> ++------------------+-----------------------------------------------------------+
> +|Return values                                                                 |
> ++------------------+-----------------------------------------------------------+
> +|Name              |Description                                                |
> ++------------------+-----------------------------------------------------------+
> +|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
> +|                  |by the chosen algorithm string.                            |
> +|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
> +|                  |matches.

So there is a fixed list from the above statement IIUC. So kindly list them
here in this document. I may need to follow the last version but I would
prefer you to explicitly mention how is the MEMLAT protocol in the previous
version gets used here ? I just can't understand that from these description.

> +
> +QCOM_SCMI_START_ACTIVITY
> +~~~~~~~~~~~~~~~~~~~~~~~~
> 

What is this activity ? How will the firmware know what is that ? All possible
details please if there are all hidden in the buffer too.
diff mbox series

Patch

diff --git a/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
new file mode 100644
index 000000000000..141bc932e30f
--- /dev/null
+++ b/drivers/firmware/arm_scmi/vendors/qcom/qcom_generic.rst
@@ -0,0 +1,211 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+.. include:: <isonum.txt>
+
+===============================================================================
+QCOM System Control and Management Interface(SCMI) Vendor Protocols Extension
+===============================================================================
+
+:Copyright: |copy| 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+
+:Author: Sibi Sankar <quic_sibis@quicinc.com>
+
+SCMI_GENERIC: System Control and Management Interface QCOM Generic Vendor Protocol
+==================================================================================
+
+This protocol is intended as a generic way of exposing a number of Qualcomm
+SoC specific features through a mixture of pre-determined algorithm string and
+param_id pairs hosted on the SCMI controller. It implements an interface compliant
+with the Arm SCMI Specification with additional vendor specific commands as
+detailed below.
+
+Commands:
+_________
+
+PROTOCOL_VERSION
+~~~~~~~~~~~~~~~~
+
+message_id: 0x0
+protocol_id: 0x80
+
++---------------+--------------------------------------------------------------+
+|Return values                                                                 |
++---------------+--------------------------------------------------------------+
+|Name           |Description                                                   |
++---------------+--------------------------------------------------------------+
+|int32 status   |See ARM SCMI Specification for status code definitions.       |
++---------------+--------------------------------------------------------------+
+|uint32 version |For this revision of the specification, this value must be    |
+|               |0x10000.                                                      |
++---------------+--------------------------------------------------------------+
+
+PROTOCOL_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x1
+protocol_id: 0x80
+
++---------------+--------------------------------------------------------------+
+|Return values                                                                 |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|int32 status      |See ARM SCMI Specification for status code definitions.    |
++------------------+-----------------------------------------------------------+
+|uint32 attributes |Bits[31:16] Reserved, must be to 0.                        |
+|                  |Bits[15:8] Number of agents in the system                  |
+|                  |Bits[7:0] Number of vendor protocols in the system         |
++------------------+-----------------------------------------------------------+
+
+PROTOCOL_MESSAGE_ATTRIBUTES
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x2
+protocol_id: 0x80
+
++---------------+--------------------------------------------------------------+
+|Return values                                                                 |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|int32 status      |See ARM SCMI Specification for status code definitions.    |
++------------------+-----------------------------------------------------------+
+|uint32 attributes |For all message id's the parameter has a value of 0.       |
++------------------+-----------------------------------------------------------+
+
+QCOM_SCMI_SET_PARAM
+~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x10
+protocol_id: 0x80
+
++------------------+-----------------------------------------------------------+
+|Parameters                                                                    |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|uint32 ext_id     |Reserved, must be zero.                                    |
++------------------+-----------------------------------------------------------+
+|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 param_id   |Serves as the token message id for the algorithm string    |
+|                  |and is used to set various parameters supported by it.     |
++------------------+-----------------------------------------------------------+
+|uint32 buf[]      |Serves as the payload for the specified param_id and       |
+|                  |algorithm string pair.                                     |
++------------------+-----------------------------------------------------------+
+|Return values                                                                 |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
+|                  |by the chosen algorithm string.                            |
+|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
+|                  |matches.                                                   |
+|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
+|                  |is rejected by the algorithm string.                       |
++------------------+-----------------------------------------------------------+
+
+QCOM_SCMI_GET_PARAM
+~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x11
+protocol_id: 0x80
+
++------------------+-----------------------------------------------------------+
+|Parameters                                                                    |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|uint32 ext_id     |Reserved, must be zero.                                    |
++------------------+-----------------------------------------------------------+
+|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 param_id   |Serves as the token message id for the algorithm string.   |
++------------------+-----------------------------------------------------------+
+|uint32 buf[]      |Serves as the payload and store of value for the specified |
+|                  |param_id and algorithm string pair.                        |
++------------------+-----------------------------------------------------------+
+|Return values                                                                 |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|int32 status      |SUCCESS: if the param_id and buf[] is parsed successfully  |
+|                  |by the chosen algorithm string and the result is copied    |
+|                  |into buf[].                                                |
+|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
+|                  |matches.                                                   |
+|                  |INVALID_PARAMETERS: if the param_id and the buf[] passed   |
+|                  |is rejected by the algorithm string.                       |
++------------------+-----------------------------------------------------------+
+
+QCOM_SCMI_START_ACTIVITY
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x12
+protocol_id: 0x80
+
++------------------+-----------------------------------------------------------+
+|Parameters                                                                    |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|uint32 ext_id     |Reserved, must be zero.                                    |
++------------------+-----------------------------------------------------------+
+|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 param_id   |Serves as the token message id for the algorithm string    |
+|                  |and is generally used to start the activity performed by   |
+|                  |the algorithm string.                                      |
++------------------+-----------------------------------------------------------+
+|uint32 buf[]      |Serves as the payload for the specified param_id and       |
+|                  |algorithm string pair.                                     |
++------------------+-----------------------------------------------------------+
+|Return values                                                                 |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|int32 status      |SUCCESS: if the activity performed by the algorithm string |
+|                  |starts successfully.                                       |
+|                  |NOT_SUPPORTED: if the algorithm string does not have any.  |
+|                  |matches or if the activity is already running.             |
++------------------+-----------------------------------------------------------+
+
+QCOM_SCMI_STOP_ACTIVITY
+~~~~~~~~~~~~~~~~~~~~~~~
+
+message_id: 0x13
+protocol_id: 0x80
+
++------------------+-----------------------------------------------------------+
+|Parameters                                                                    |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|uint32 ext_id     |Reserved, must be zero.                                    |
++------------------+-----------------------------------------------------------+
+|uint32 algo_low   |Lower 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 algo_high  |Upper 32-bit value of the algorithm string.                |
++------------------+-----------------------------------------------------------+
+|uint32 param_id   |Serves as the token message id for the algorithm string    |
+|                  |and is generally used to stop the activity performed by    |
+|                  |the algorithm string.                                      |
++------------------+-----------------------------------------------------------+
+|uint32 buf[]      |Serves as the payload for the specified param_id and       |
+|                  |algorithm string pair.                                     |
++------------------+-----------------------------------------------------------+
+|Return values                                                                 |
++------------------+-----------------------------------------------------------+
+|Name              |Description                                                |
++------------------+-----------------------------------------------------------+
+|int32 status      |SUCCESS: if the activity performed by the algorithm string |
+|                  |stops successfully.                                        |
+|                  |NOT_SUPPORTED: if the algorithm string does not have any   |
+|                  |matches or if the activity isn't running.                  |
++------------------+-----------------------------------------------------------+