Re: [PATCH v3 1/7] firmware: arm_scmi: imx: Add LMM and CPU documentation

[Sudeep Holla]

On Thu, Apr 03, 2025 at 12:10:37AM +0800, Peng Fan wrote:
> Hi Sudeep,
> 
> On Wed, Apr 02, 2025 at 12:46:14PM +0100, Sudeep Holla wrote:
> >On Wed, Apr 02, 2025 at 08:35:03PM +0800, Peng Fan wrote:
> >> Hi Sudeep,
> >> 
> >> Thanks for reviewing the patch.
> >> 
> >> For comments that I am not very clear, I marked with [TODO] for easily
> >> jump to.
> >> 
> >> On Tue, Apr 01, 2025 at 03:15:46PM +0100, Sudeep Holla wrote:
> >> >On Mon, Mar 03, 2025 at 10:53:22AM +0800, Peng Fan (OSS) wrote:
> >> >> From: Peng Fan <peng.fan@xxxxxxx>
> >> >> 
> >> >> Add i.MX95 Logical Machine Management and CPU Protocol documentation.
> >> >> 
> >> >> Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> >> >> ---
> >> >>  drivers/firmware/arm_scmi/vendors/imx/imx95.rst | 801 ++++++++++++++++++++++++
> >> >>  1 file changed, 801 insertions(+)
> >> >> 
> >> >> diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx95.rst b/drivers/firmware/arm_scmi/vendors/imx/imx95.rst
> >> >> index b2dfd6c46ca2f5f12f0475c24cb54c060e9fa421..74326bf2ea8586282a735713e0ab7eb90ccce8ff 100644
> >> >> --- a/drivers/firmware/arm_scmi/vendors/imx/imx95.rst
> >> >> +++ b/drivers/firmware/arm_scmi/vendors/imx/imx95.rst
> >
> >> >> +
> >> >> +PROTOCOL_MESSAGE_ATTRIBUTES
> >> >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> +
> >> >> +message_id: 0x2
> >> >> +protocol_id: 0x80
> >> >> +This command is mandatory.
> >> >> +
> >> >
> >> >For completeness add parameters here for message_id as in the spec as it is
> >> >referred in the returned value and seems incomplete without it.
> >> 
> >> [TODO]
> >> Sorry, I may not get your point here. You mean below format?
> >> 
> >> +------------------+-----------------------------------------------------------+
> >> |message_id: 0x2
> >> |protocol_id: 0x80
> >> |This command is mandatory.
> >> +------------------+-----------------------------------------------------------+
> >> |Return values                                                                 |
> >> +------------------+-----------------------------------------------------------+
> >> |Name              |Description                                                |
> >> +------------------+-----------------------------------------------------------+
> >> |int32 status      |SUCCESS: in case the message is implemented and available  |
> >> |                  |to use.                                                    |
> >> |                  |NOT_FOUND: if the message identified by message_id is      |
> >> |                  |invalid or not implemented                                 |
> >> +------------------+-----------------------------------------------------------+
> >> |uint32 attributes |Flags that are associated with a specific function in the  |
> >> |                  |protocol. For all functions in this protocol, this         |
> >> 
> >> message_id is not put in the table, but it is list above just below
> >> the protocol name. I would prefer to keep current layout and align with
> >> the MISC and BBM protocol.
> >>
> >
> >I meant why is the input parameter message_id not described in the table,
> >but is referred in the return values. For completeness, just add it even
> >though it may match the SCMI spec in terms of input parameter.
> 
> I will add below only for PROTOCOL_MESSAGE_ATTRIBUTES which refer message_id
> in the return values. Please raise if you have concern.
> 

Ignore this. I see even existing BBM and MISC follow the same pattern for
standard protocol commands(0x0-0x2). We can fix them all at once if it
needs to be in the future. For now, it should be fine as is.

-- 
Regards,
Sudeep