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

Sudeep Holla
2025-04-02T15:30:35Z

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@nxp.com>
> >> >>
> >> >> Add i.MX95 Logical Machine Management and CPU Protocol documentation.
> >> >>
> >> >> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >> >> ---
> >> >> 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